Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add min, max where appropriate. #3651

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HansOlsson
Copy link
Collaborator

Closes #3650

I'm not sure if using the table in this way is ideal, if not I could see us adding a short description in chapter 3.7 forwarding to the other variant. I don't think that moving those two variants, and duplicating the actual definition would be bad.

@@ -440,7 +442,8 @@ \subsection{Numeric Functions and Conversion Operators}\label{numeric-functions-
\end{tabular}
\end{center}

All of these except for the \lstinline!String! conversion operator are vectorizable according to \cref{scalar-functions-applied-to-array-arguments}.
Except for the \lstinline!min!, \lstinline!max! and the \lstinline!String! conversion operator they are vectorizable according to \cref{scalar-functions-applied-to-array-arguments}.
The \lstinline!min! and \lstinline!max! functions including additional variants are described in \cref{reduction-functions-and-operators}.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find including additional variants a bit unclear, could it be reformulated?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The \lstinline!min! and \lstinline!max! functions have array-specific variants which perform array reduction operations described in ...

Comment on lines +434 to +435
{\lstinline!min($x$, $y$)!} & Least of two scalars & \Cref{modelica:min-binary} \\
{\lstinline!max($x$, $y$)!} & Greatest of two scalars & \Cref{modelica:max-binary} \\
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the entries above:

Suggested change
{\lstinline!min($x$, $y$)!} & Least of two scalars & \Cref{modelica:min-binary} \\
{\lstinline!max($x$, $y$)!} & Greatest of two scalars & \Cref{modelica:max-binary} \\
{\lstinline!min($x$, $y$)!} & Least of two scalars (event-free) & \Cref{modelica:min-binary} \\
{\lstinline!max($x$, $y$)!} & Greatest of two scalars (event-free) & \Cref{modelica:max-binary} \\

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move min(x,y) and max(x,y) definition out of array functions
3 participants