Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add rule for allowing fixed parameters to be unbound during translation #3075
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 rule for allowing fixed parameters to be unbound during translation #3075
Changes from 1 commit
6ca3f77
7a1fdb2
326a194
46cb560
b17cf77
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just remove this line, as it seems like such an odd idea to guard against.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is an important point, which is actually not clear in the specification right now. What is meant by "providing start-attribute"? Consider the following example:
I tried it in MapleSim, Dymola and OpenModelica. It simulates, giving a warning that the default values is used. But is it correct to consider that the start value is given? I scanned through units in MSL. There is only ThermodynamicTemperature that has start value specified on type. But then there is a bunch of types that extend from it. So, if somebody has a parameter of ThermodynamicTemperature type and does not specify the start attribute when that instance is defined (and no modification given at any point later), do we consider that the start-attribute is provided or not? With the sentence that Henrik added, it should not be considered as provided. But it is treated as such right now but at least 3 tools. If that sentence is not correct then it should be stated that it is enough to specify the start on the type, but start values of builtin types should not be considered as "provided".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me, this line is at the heart of the proposal. I don't want this proposal to end up removing the current rule that all parameters must be given a value – either directly or using the
start
attribute to bypass the rule.In @eshmoylova's example above, the intention is to consider
p
as havingstart
-attribute, meaning that the sentence we're discussing here doesn't apply. Hence, I don't understand this comment:There should be no special rule about being considered "provided". Provide
start
, and you bypass the mechanism to guard against use of uninitialized parameters. To ensure that parameters are given values by users of a model one must not providestart
, neither directly nor via the use of a type alias.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of discussing "modifier for start-attribute" I believe we should:
RealStart=Real(start=0)
has a modifier for the start-attribute, but Real withRealType start=0;
lacks it, seems weird. This is also consistent with this PR removing the "non-zero" text.We also have to consider the following odd cases for start-attributes (assuming MCP0009 is accepted):
Both of them arguably has a modifier for the start-attribute, but neither has a value for the start-attribute.
(Using final on a modifier without providing a value is well-defined semantically, but normally it is used when you already have a value, in order to finalize that.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hoping for some feedback on removing this.
Additionally I realized that apart from being an odd idea, I don't understand what it means.
How do we determine the default value for the parameter's type? The obvious idea would be to use the start-value of the type, but in that case the parameter has a start-value and the sentence does not apply.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this would need to be discussed in a separate issue to avoid scope creep. Some things to keep in mind when working out such a proposal:
start = break
is not a modifier, but removes an existing modifier.RealType start = 0
can come into play.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gave feedback above: #3075 (comment)
The purpose is to not break a big hole in the current safety net for ensuring that all parameters must have a value when a model is simulated.