Skip to content

Conversation

henrikt-ma
Copy link
Collaborator

@henrikt-ma henrikt-ma commented Jan 29, 2025

Based on discussion in #3631.

This restores the Modelica 3.6 (latest release) situation of the absoluteValue-annotation not having a constant default value. The difference to the 3.6 situation is that the default behavior is described in the text, as being tool inference. Similar to unit checking, the specification does not give details about how the inference shall be performed.

Without directly recommending for or against explicit use of absoluteValue, the non-normative text is still biased by only explaining possible consequences of leaving it out, and not mentioning the drawbacks of over-specifying. This is meant to be a compromise between fear of problems when using tools that don't have good inference, and fear of pedantic/ambitious library authors putting the annotation all over their libraries in ways that might do more harm than good when using tools with good inference.

Fixes #3631.

@HansOlsson
Copy link
Collaborator

To me it doesn't make sense to specify that it is inferred without specifying how it is inferred (which to me requires unit-inferrence as we don't even know which variables require it otherwise).

That's why I suggested:

For a component where unit conversions involving non-zero offset is possible (mainly temperatures) it is recommend to give a value for absoluteValue in annotation either for the component or the type declaration. Tools may infer absoluteValue in other cases, but specifying absoluteValue reduces impact of quality-of-implementation in tool ability to infer absoluteValue.

That still gives the possibility to infer, but without making models too reliant on it.

@HansOlsson
Copy link
Collaborator

I believe the fear of over-using the annotation is misplaced. To me it is a matter of using Modelica.Units.SI.Temperature or Modelica.Units.SI.TemperatureDifference.

If we consider parameters of a model I believe it is good style to declare them with a type (that includes unit and absoluteValue-annotation). Similarly for physical connectors. I would also consider it normal to declare other variables in physical models to be declared with a type (including unit and absoluteValue).

The exception I see is when you build blocks interacting with such models (using sensors and sources), where I would see it as unusual to have a unit and absoluteValue (looking more I realize that there are some that have unit="K" - most, but not all implicitly have absoluteValue=true - and the exception I found was wrong in other ways as well - will open an issue).

Copy link
Collaborator

@HansOlsson HansOlsson left a comment

Choose a reason for hiding this comment

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

Having inference as the default without specifying it is an issue.

@HansOlsson
Copy link
Collaborator

It is also important that any such inference is restricted to the cases where it makes sense to avoid problems. The ones that are clear are:

  • Two-terms: Equality (a=b)
  • Three-terms (regardless of written as addition or subtraction a=b-c, b=a+c):
    • The difference between two absolute temperatures is a relative one.
    • Adding/subtracting two relative temperatures give a relative one.

The reason to be restrictive is that if tools add additional incorrect rules there might be a push to update existing good models in bad ways to circumvent those checks. Since such a scenario is not just hypothetical, I don't see how we can add inference as default without specifying the how.

@HansOlsson HansOlsson added this to the 2025-February milestone Feb 5, 2025
@HansOlsson
Copy link
Collaborator

BTW: Such an inference has been test-implemented in Dymola, and correctly handles the sensors for absolute and relative temperature.

@henrikt-ma
Copy link
Collaborator Author

It is also important that any such inference is restricted to the cases where it makes sense to avoid problems. The ones that are clear are:

The rules for unit checking are also important. This PR is primarily about restoring the 3.6 situation where the specification did not rule out the possibility of doing some sort of inference, but instead of just going back to the unexplained default in 3.6, this PR goes a little further by also pointing out that tools need to be aware of when their inference isn't conclusive. Similar to unit inference, we need to have faith in tools until we manage agree upon exactly how absoluteValue inference should be done.

@HansOlsson
Copy link
Collaborator

It is also important that any such inference is restricted to the cases where it makes sense to avoid problems. The ones that are clear are:

The rules for unit checking are also important. This PR is primarily about restoring the 3.6 situation where the specification did not rule out the possibility of doing some sort of inference, but instead of just going back to the unexplained default in 3.6, this PR goes a little further by also pointing out that tools need to be aware of when their inference isn't conclusive. Similar to unit inference, we need to have faith in tools until we manage agree upon exactly how absoluteValue inference should be done.

But:

  • For units we don't tell users that inference is the default, instead we only define explicit unit-attribute in the specification. Why should we do it differently for absoluteValue based on some future work?
  • One can neither completely boot-strap unit nor absoluteValue for a model, inference starts from some variables with units and infer units for other variables.
  • For many the focus is on checking not inference; and to me that is a valid approach; by stating that inference is the default we demote that.

In practice if we look at MSL:

  • In Modelica.Blocks.Examples.Noise.Utilities.Parts.MotorWithCurrentControl (and similar ones) we can infer unit and absoluteValue for const-blocks in smpm.thermalAmbient - a nice to have, but nothing more.
  • But in Modelica.Clocked.Examples.Systems.ControlledMixingUnit I don't see how we can infer absoluteValue=true for the different temperatures in the mixing unit. The clearest indicator that it is an absolute value is exp( -weps/T), but that is really stretching it.

Co-authored-by: Henrik Tidefelt <henrikt@wolfram.com>
@HansOlsson HansOlsson merged commit 6bcd97a into modelica:master Feb 27, 2025
1 check passed
@henrikt-ma henrikt-ma deleted the absoluteValue-default branch February 27, 2025 12:04
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.

Default for absoluteValue

2 participants