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

Jump to bottom

Can AssertionLevel change dynamically? #2642

Closed
maltelenz opened this issue Sep 1, 2020 · 7 comments · Fixed by #2698
Closed

Can AssertionLevel change dynamically? #2642

maltelenz opened this issue Sep 1, 2020 · 7 comments · Fixed by #2698
Milestone
Phone2020-5

Comments

@maltelenz
Copy link
Collaborator

In ModelicaCompliance.Equations.Assert.AssertVarLevel we have:

model AssertVarLevel
  Real x(start = 0);
equation
  x = time;
  assert(x < 0.5, "This assert should abort the simulation at time 0.6.", if x > 0.6 then AssertionLevel.error else AssertionLevel.warning);
  annotation(__ModelicaAssociation(TestCase(shouldPass = false, section = {"8.3.7"})), experiment(StopTime = 1), Documentation(info = "<html>Checks that the assertion level can itself be dependent on a
        condition.</html>"));
end AssertVarLevel;

How should one read this test?

A: The shouldPass = false means this is against the specification, and a tool should reject it.
B: The documentation string and and the assert message describe what is expected to happen, and a tool should handle this model, issuing first a warning at time = 0.5, then an error at time = 0.6.

I would vote for A, and only allow literals in the level of an assert. The example at the end of 8.3.7 seems to indicate using two separate asserts for this use case is the recommended way.

If we allow B, what happens to the implicit noEvent in the warning case? Does it have to be dynamically switched off and on as the level changes? #2571 seems relevant as well.
@HansOlsson
Copy link
Collaborator

The example is a bit silly.
However, there is nothing in the specification stating that the assertionLevel must be a literal; so I don't see anything supporting A.
Thus the conclusion is that the answer is B.

Obviously this influences the noEvent-handling, but that is already being discussed with no consensus and a unevaluated parametric assertionLevel have the similar issue; as noted in #2571

@maltelenz
Copy link
Collaborator Author

To be a bit contrary, there is nothing in the specification stating that the assertion level can be anything but a literal either. In fact, the specification only explains what happens if level = AssertionLevel.error and level = AssertionLevel.warning. It does not (explicitly) cover the case where level = expr and expr evaluates to either of the two literals, nor does the text mention this is a possibility.

The example at the end of the section does show that the non-literal case is not needed, and seems much cleaner than throwing in more conditions in something that is not the condition argument.

@gkurzbach
Copy link
Collaborator

I have nothing against an expression at assertionLevel, but this must be evaluatable at compile time, in general it should have at least parameter variability.

The reason is, that the compiler can handle both cases differently. In case of AssertionLevel.error it needs to be able to control the solver. Therefore the condition must be evaluated each time the equations are calculated. In case of AssertionLevel.warning, the condition must be evaluated only if the current step is valid, because only then warnings should be issued to avoid cluttering the output. Therefore the generated code for warnings could be placed in a different section to not produce an overhead when solving the system of equations.

@henrikt-ma
Copy link
Collaborator

Right, I think the intention here was to clarify that the assertion level is a "structural parameter".

@HansOlsson
Copy link
Collaborator

To be a bit contrary, there is nothing in the specification stating that the assertion level can be anything but a literal either. In fact, the specification only explains what happens if level = AssertionLevel.error and level = AssertionLevel.warning. It does not (explicitly) cover the case where level = expr and expr evaluates to either of the two literals, nor does the text mention this is a possibility.

To me specifying that arguments are evaluated for all primitives seem a bit unnecessary, and we should mostly assume that they are evaluated. Assert also says "If the condition of an assertion is true, message is not evaluated and the procedure call is ignored." It doesn't say "If the condition of an assertion evaluates to true."

Right, I think the intention here was to clarify that the assertion level is a "structural parameter".

We can certainly state that. I don't see any major problems with it.

@maltelenz
Copy link
Collaborator Author

We can certainly state that. I don't see any major problems with it.

Sounds good. This would make the originally given example invalid, since it has an assertion level with a higher variability than parameter.

@HansOlsson HansOlsson added this to the Phone2020-5 milestone Oct 26, 2020
@HansOlsson
Copy link
Collaborator

Poll (can be in favor of more than one):

  • No change: Hans, Axel, Filippo, Leo
  • Required to be structural parameter: Hans, Christoff, Gerd, Henrik, Leo, Martin, Quentin, Stephan
  • Required to be constant: Gerd, Henrik, Martin, Quentin
    Rest abstain
    Conclusion: structural parameter

@henrikt-ma henrikt-ma mentioned this issue Nov 2, 2020
Merged
@maltelenz maltelenz mentioned this issue May 15, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Phone2020-5
Development

Successfully merging a pull request may close this issue.

Make assertion level a structural parameter henrikt-ma/ModelicaSpecification
4 participants
@maltelenz @HansOlsson @henrikt-ma @gkurzbach