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.

Sign up for GitHub

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

Start clearing up annotation syntax for functions. #3617

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Start clearing up annotation syntax for functions. #3617

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e34ad0e
Start clearing up annotation syntax for functions.
HansOlsson Nov 29, 2024
3f1bc07
Apply suggestions from code review
HansOlsson Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions chapters/functions.tex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1670,6 +1670,7 @@ \subsection{Using the Inverse Annotation}\label{using-the-inverse-annotation}
\section{Function Inlining and Event Generation}\label{function-inlining-and-event-generation}

The annotations listed below affect inlining of functions and the related topic of event generation inside functions.
See \cref{notation-for-annotation-definitions} regarding the notation used to describe the annotations.
\begin{center}
\begin{tabular}{l|l l}
\hline
Expand All @@ -1689,8 +1690,8 @@ \section{Function Inlining and Event Generation}\label{function-inlining-and-eve
Hence, one needs to find the right balance between inlining too early (loss of provided derivatives and inverses) and too late (earlier stages of symbolic processing cannot benefit from symbolic simplifications).

\begin{annotationdefinition}[Inline]
\begin{synopsis}[grammar]\begin{lstlisting}
"Inline" "=" ( false | true )
\begin{synopsis}\begin{lstlisting}
/*literal*/ constant Boolean Inline;
\end{lstlisting}\end{synopsis}
\begin{semantics}
Has only an effect within a function declaration.
Expand All @@ -1700,15 +1701,17 @@ \section{Function Inlining and Event Generation}\label{function-inlining-and-eve

If {\lstinline!Inline = false!}, the model developer proposes to not inline the function.

The default for inlining is tool-specific.

\begin{nonnormative}
{\lstinline!Inline = true!} is for example used in {\lstinline!Modelica.Mechanics.MultiBody.Frames!} and in functions of {\lstinline!Modelica.Media!} to have no overhead for function calls such as resolving a vector in a different coordinate system and at the same time the function can be analytically differentiated, e.g., for index reduction needed for mechanical systems.
\end{nonnormative}
\end{semantics}
\end{annotationdefinition}

\begin{annotationdefinition}[LateInline]
\begin{synopsis}[grammar]\begin{lstlisting}
"LateInline" "=" ( false | true )
\begin{synopsis}\begin{lstlisting}
/*literal*/ constant Boolean LateInline = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought we decided to avoid writing out defaults like this, as it gives the impression that LateInline = false is what one is supposed to write?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As I recall the idea was not use the form when specifying the default literal value (assuming it exists), when it is so "normal" that specifying it would look like an error (like singleInstance).

Different inlining variant and GenerateEvents don't fall under that; and is more similar to absoluteValue.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's not how I recall it. For me, the main problem which was resolved when removing all those defaults was that what the reader sees first shouldn't be something one normally doesn't write when using the annotation. This was avoided by drastically reducing the use of declaration equations for explaining defaults in case of component style annotations.

I find LateInline similar to DocumentationClass, for which the default was removed in aa6ce47.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To me these two explanations are fairly similar.
We agree that using DocumentationClass = false does not really make sense at all in practice, the point is that LateInline = false is just a bit unusual - one can easily see someone experimenting with setting LateInline to true or false to test the result.

And most of the removed defaults in that commit weren't literal values, but explanations as defaults.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, so we have different recollections of the exact formulations used in the meeting discussions behind removing literal defaults. Fine.

Now, if we think about how to write clear specification text, don't you agree that there is a problem with picking the style of presentation based on vague opinions about what is more or less unusual or odd? … that specifying defaults for component style annotations sometimes with declaration equation and sometimes in text (even when the default corresponds to a literal value), is causing more confusion than useful brevity?

\end{lstlisting}\end{synopsis}
\begin{semantics}
Has only an effect within a function declaration.
Expand All @@ -1735,8 +1738,8 @@ \section{Function Inlining and Event Generation}\label{function-inlining-and-eve
\end{annotationdefinition}

\begin{annotationdefinition}[InlineAfterIndexReduction]
\begin{synopsis}[grammar]\begin{lstlisting}
"InlineAfterIndexReduction" "=" ( false | true )
\begin{synopsis}\begin{lstlisting}
/*literal*/ constant Boolean InlineAfterIndexReduction = false;
\end{lstlisting}\end{synopsis}
\begin{semantics}
Has only an effect within a function declaration.
Expand All @@ -1747,8 +1750,8 @@ \section{Function Inlining and Event Generation}\label{function-inlining-and-eve
\end{annotationdefinition}

\begin{annotationdefinition}[GenerateEvents]
\begin{synopsis}[grammar]\begin{lstlisting}
"GenerateEvents" "=" ( false | true )
\begin{synopsis}\begin{lstlisting}
/*literal*/ constant Boolean GenerateEvents = false;
\end{lstlisting}\end{synopsis}
\begin{semantics}
Has only an effect within a function declaration.
Expand Down