modelica
Added assert condition block
- Block is useful for applying asserts via graphical layer (rather than via equations)
Investigating origin:
TerminateSimulation was added in https://github.com/modelica/ModelicaStandardLibrary/commit/952520dc9fc8c89c511cbde7bcf4e4b050c7504f (2007-03-25)
And BooleanExpression in https://github.com/modelica/ModelicaStandardLibrary/commit/ee9a1a29568c2815a0e568719f25482a50d388c7 (2004-07-07 - MSL 2.1Beta1 )
The block has been restructured based on the previous discussion. Note that the initial scale of the block is 0.04 to be in alignment with the default height of the BooleanExpression block, which this block is expected to be paired with typically. See the picture below (top is the default rather than the bottom):
Note that the initial scale of the block is 0.04 to be in alignment with the default height of the BooleanExpression block, which this block is expected to be paired with typically. See the picture below (top is the default rather than the bottom):
Instead of using unusual initialScale, I think the best way to get more consistent font sizes is to stick to what other icons to (typically 0.1), and instead reduce the extent.
That said, I think I'd prefer the full 20x20 size, although the text assert should probably be made smaller to look nice.
Note that the initial scale of the block is 0.04 to be in alignment with the default height of the BooleanExpression block, which this block is expected to be paired with typically. See the picture below (top is the default rather than the bottom):
Instead of using unusual
initialScale, I think the best way to get more consistent font sizes is to stick to what other icons to (typically 0.1), and instead reduce theextent.That said, I think I'd prefer the full 20x20 size, although the text assert should probably be made smaller to look nice.
A couple notes here:
- Since I was extending from a partial model, it seemed easier to use initialScale approach. Various other blocks in this part of the library also used that approach (Modelica.Blocks.Interfaces.PartialRealMISO, Modelica.Blocks.Interfaces.PartialBooleanSISO_small)
- For the text, I also followed the precedent for all other blocks in Modelica.Blocks.Logical, which had the same text extent: {{-90,40},{90,-40}}. I could drop it down to +- 85 or 80, but it seemed more aligned to just copy the precedent already set. Note that Modelica.Blocks.Logical.FallingEdge has a similarly long word (falling) with the same text extent.
I'm open to changes, but just wanted to make you aware of why I made the choices I did. Basically they aren't new choices, just following previous examples and choices already made.
Since I was extending from a partial model, it seemed easier to use initialScale approach.
An option for icon could be to disable the icon of partial model in extends clause by annotation (IconMap(primitivesVisible=false, extent={{-100,-100}, {100,100}})) and to define it similar to boolean expression (size, scaling, graphical elements).
I'm open to changes, but just wanted to make you aware of why I made the choices I did. Basically they aren't new choices, just following previous examples and choices already made.
OK, thanks for explaining the design. Matching the style of FallingEdge makes sense, and then my personal preference would simply be to also use the same initialScale. By doing so, font sizes will be at least somewhat consistent with other icons such as FallingEdge.
- For the text, I also followed the precedent for all other blocks in Modelica.Blocks.Logical, which had the same text extent: {{-90,40},{90,-40}}. I could drop it down to +- 85 or 80, but it seemed more aligned to just copy the precedent already set. Note that Modelica.Blocks.Logical.FallingEdge has a similarly long word (falling) with the same text extent.
The problem with the extent, as I see it, is that text gets scaled to fill the width rather than the height, meaning that one gets all sorts of effective font sizes depending on what goes into the box. Anyway, I don't think this is the PR where we're supposed to come up with new icon text design guidelines, so copying an existing style is probably the best way to proceed.
Ok, so based on the discussion I went ahead and removed the initial scale annotation. Now by default the scale will match the 2nd example in the picture above.
I think the PR should be good to go at this point unless any additional concerns.
@beutlich Is it allowed to link to ModelicaReference.Operators.'assert()' in the documentation? This could be helpful since the component fully use it.
I have changed the default of assertionLevel paremeter since I believe the component will mostly be used to trigger a warning, not to terminate the simulation completely.
I have changed the default of
assertionLevelparemeter since I believe the component will mostly be used to trigger a warning, not to terminate the simulation completely.
And you're sure it wouldn't make more sense to use the same default as the Modelica language assert?
@beutlich How about https://github.com/modelica/ModelicaStandardLibrary/pull/3985#issuecomment-1203712017 ?
@beutlich Is it allowed to link to
ModelicaReference.Operators.'assert()'in the documentation? This could be helpful since the component fully use it.
Why not? See also #2108.
Made the minor changes as requested (documentation paragraph changes, removal of getInstanceName(), and changing the wording from terminate to abort). Let me know if anything else, otherwise, looking to approve and merge this this here.