Skip to content

Incomplete release notes #4601

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
HansOlsson opened this issue Mar 24, 2025 · 13 comments
Closed

Incomplete release notes #4601

HansOlsson opened this issue Mar 24, 2025 · 13 comments
Labels
L: UsersGuide Issue addresses Modelica.UsersGuide V: 4.1.0-dev Issue originates in MSL v4.1.0-dev (and is not present in earlier releases)
Milestone

Comments

@HansOlsson
Copy link
Contributor

#3627 is not listed under "non-backwards compatible" changes, but I believe it should be (as existing models might break).

@HansOlsson HansOlsson added the V: 4.1.0-dev Issue originates in MSL v4.1.0-dev (and is not present in earlier releases) label Mar 24, 2025
@HansOlsson HansOlsson added this to the MSL4.1.0 milestone Mar 24, 2025
@HansOlsson
Copy link
Contributor Author

Similarly for #4042

(Both of these are bug-fixes, and good in themselves - but they might still cause user models to break.)

@HansOlsson
Copy link
Contributor Author

HansOlsson commented Mar 24, 2025

Text along the lines of:

@beutlich beutlich added the L: UsersGuide Issue addresses Modelica.UsersGuide label Mar 25, 2025
@GallLeo
Copy link
Contributor

GallLeo commented Mar 25, 2025

  • Modelica.Contants,{eps,...}

The changed Modelica.Constants will probably break some user models, so it should be mentioned in the top summary of the release notes.

I'll try to make the changes easier to compare/show:

Definition in ModelicaServices 4.0.0:
https://github.com/modelica/ModelicaStandardLibrary/blob/maint/4.0.x/ModelicaServices/package.mo#L180

  package Machine "Machine dependent constants"
    extends Modelica.Icons.Package;
    final constant Real eps=1e-15 "Biggest number such that 1.0 + eps = 1.0";
    final constant Real small=1e-60 "Smallest number such that small and -small are representable on the machine";
    final constant Real inf=1e60 "Biggest Real number such that inf and -inf are representable on the machine";
    final constant Integer Integer_inf=2147483647 "Biggest Integer number such that Integer_inf and -Integer_inf are representable on the machine";

Definition in ModelicaServices 4.1.0:
https://github.com/modelica/ModelicaStandardLibrary/blob/maint/4.1.x/ModelicaServices/package.mo#L179

package Machine "Machine dependent constants"
    extends Modelica.Icons.Package;
    final constant Real eps=2.2204460492503131e-016 "The difference between 1 and the least value greater than 1 that is representable in the given floating point type";
    final constant Real small=2.2250738585072014e-308 "Minimum normalized positive floating-point number";
    final constant Real inf=1.7976931348623157E+308 "Maximum representable finite floating-point number";
    final constant Integer Integer_inf=2147483647 "Biggest Integer number such that Integer_inf and -Integer_inf are representable on the machine";
...
name MSL 4.0.0 MSL 4.1.0
eps 1e-15 2.2204460492503131e-016
small 1e-60 2.2250738585072014e-308
inf 1e60 1.7976931348623157E+308
Integer_inf 2147483647 2147483647

To be clarified for release notes:

@HansOlsson
Copy link
Contributor Author

name MSL 4.0.0 MSL 4.1.0
eps 1e-15 2.2204460492503131e-016
small 1e-60 2.2250738585072014e-308
inf 1e60 1.7976931348623157E+308
Integer_inf 2147483647 2147483647
To be clarified for release notes:

  • Are the values fixed for all tools? Or might tool vendors or computer architecture change the actual values?

They are fixed if using IEEE double precision floating numbers - which is what almost every current computer uses (which is the minimum recommended floating point number in Modelica).

I would say that the previous values were just placeholders, and that the description texts problematic.

@beutlich
Copy link
Member

beutlich commented Mar 26, 2025

The changed Modelica.Constants will probably break some user models

Yes, we already know about broken models: See #4478.

@HansOlsson
Copy link
Contributor Author

The changed Modelica.Constants will probably break some user models

Yes, we already know about broken models: See #4478.

See also the longer list in #2056
(and the reason for this issue was yet more models).

In particular:

@GallLeo
Copy link
Contributor

GallLeo commented Mar 31, 2025

I see that there are benefits of the new definition.

But, I'm still convinced that we should not break user models with a minor version, if we can avoid it.

Could we offer a conversion script to deprecated constants?
Probably yes, but only for user models.
All MSL models or library models referencing Modelica.Constants can potentially change numerical behavior.

@HansOlsson
Copy link
Contributor Author

I see that there are benefits of the new definition.

But, I'm still convinced that we should not break user models with a minor version, if we can avoid it.

Could we offer a conversion script to deprecated constants? Probably yes, but only for user models. All MSL models or library models referencing Modelica.Constants can potentially change numerical behavior.

Tools can offer settings working arounds these changes.
Converting any use of Modelica.Constants.eps to 1e-15 might be possible for compatibility, but to me it would set a bad precedence.

Similarly some models will break due to assert added in #3627
We could for compatibility have changed the default for the flag (or possibly added that as a conversion) so that existing models would have de-activated the assert - but I don't think it would make sense.

@casella
Copy link
Contributor

casella commented Apr 1, 2025

  • Are the values fixed for all tools? Or might tool vendors or computer architecture change the actual values?

I don't want to re-start a long discussion here on why I believe the previous values were better (I believe #3390, where we had to put an awful 1e60 hard-wired literal constant instead of a much nicer Inf constant, provides another excellent demonstration of my arguments in favour of the previous definitions), because that's not the appropriate place to do it.

I'd just like to point out that the contents of the ModelicaServices.Machine package are by definition machine dependent, so tool vendors are free to replace them with whatever they seem fit. Otherwise, they woudn't belong to ModelicaServices in the first place.

@casella
Copy link
Contributor

casella commented Apr 1, 2025

#3627 is not listed under "non-backwards compatible" changes, but I believe it should be (as existing models might break).

Discussion during the MAP-Lib monthly meeting:

  • new implementation is not backwards-compatible because it fixes a bug in the original design
  • existing models could fail because of asserts. In that case the cure is to set allowOutOfRange=true in the model

@casella
Copy link
Contributor

casella commented Apr 1, 2025

Similarly for #4042

Text for the release notes:

  • definition of Modelica.Constants.inf/small/eps changed to make them consistent with commonly accepted definitions (see, e.g., Wikipedia), see PR #4042.
  • as a consequence, the ModelicaServices implementation by tool vendors may have changed compared to 4.0.0. Tool vendors may provide a compatibility flag to keep the old values.

Unanimously approved

@casella
Copy link
Contributor

casella commented Apr 11, 2025

#3627 is not listed under "non-backwards compatible" changes, but I believe it should be (as existing models might break).

It now is, see cf0e2a3.

@casella
Copy link
Contributor

casella commented Apr 11, 2025

Similarly for #4042

Text for the release notes:

* definition of Modelica.Constants.inf/small/eps changed to make them consistent with commonly accepted definitions (see, e.g., [Wikipedia](https://en.wikipedia.org/wiki/Machine_epsilon)), see [PR #4042](https://github.com/modelica/ModelicaStandardLibrary/pull/4042/files#diff-8e0ca28ecc0790866b30336c92bd31b07f279ad7d31ed3325dab67e60a1835a8).

* as a consequence, the ModelicaServices implementation by tool vendors may have changed compared to 4.0.0. Tool vendors may provide a compatibility flag to keep the old values.

Unanimously approved

Done in a6b8ba1.

@casella casella closed this as completed Apr 11, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
L: UsersGuide Issue addresses Modelica.UsersGuide V: 4.1.0-dev Issue originates in MSL v4.1.0-dev (and is not present in earlier releases)
Projects
None yet
Development

No branches or pull requests

4 participants