search

modelica / ModelicaStandardLibrary

Free (standard conforming) library to model mechanical (1D/3D), electrical (analog, digital, machines), magnetic, thermal, fluid, control systems and hierarchical state machines. Also numerical functions and functions for strings, files and streams are included.
https://doc.modelica.org
BSD 3-Clause "New" or "Revised" License
452 stars 165 forks source link

Is Modelica.Utilities.Strings.hashString supposed to be tool-independent? #4419

Closed casella closed 1 week ago

casella commented 2 weeks ago

The documentation of Modelica.Utilities.Strings.hashString says:

immagine

This is consistent with ModelicaTest.Utilities.TestString: https://github.com/modelica/ModelicaStandardLibrary/blob/83189ce507a0230b0c79ca4c30c052df8dfda3e2/ModelicaTest/Utilities.mo#L210-L211

However, if I compute Modelica.Utilities.Strings.hashString("this is a test") with Dymola 2024x I get 1734720762, not 1827717433. In fact, ModelicaTest.Utilities fails in Dymola 2024x.

Apparently, this issue was discussed in the context of #3415 when looking for MSL 3.2.3->4.0.0 regressions and eventually dismissed as a "tool issue". Looking at PR #3442, the hash test was removed on Feb 24 2020 and re-introduced on Feb 25 2020 in the PR, which was then merged on master on Jan 19 2021.

Do I understand correctly that the intent is that hashString is tool-independent? This has consequences in real life, see OpenModelica/OpenModelica#12569

beutlich commented 2 weeks ago

Do I understand correctly that the intent is that hashString is tool-independent?

Yes, this Dymola issue was finally resolved by #3728 more than 3 years ago.

However, if I compute Modelica.Utilities.Strings.hashString("this is a test") with Dymola 2024x

Which MSL version? v4.0.0 or v4.1.0-beta.1 or really 83189ce507a0230b0c79ca4c30c052df8dfda3e2?

casella commented 2 weeks ago

Yes, this Dymola issue was finally resolved by #3728 more than 3 years ago.

Not sure, see comment by @mwetter.

However, if I compute Modelica.Utilities.Strings.hashString("this is a test") with Dymola 2024x

Which MSL version? v4.0.0 or v4.1.0-beta.1 or really 83189ce?

It's 4.0.0. So I guess the problem is still there because the solution was on the MSL master after the 4.0.0 release?

beutlich commented 2 weeks ago

It was a latent issue (caused by jumbo / unity build of MSL C sources), which has been addressed library-wise by c114aeef8cb65e18f82a9a959b9e92c940654e38 more than 3 years ago.

HansOlsson commented 1 week ago

Yes, this Dymola issue was finally resolved by #3728 more than 3 years ago.

Not sure, see comment by @mwetter.

However, if I compute Modelica.Utilities.Strings.hashString("this is a test") with Dymola 2024x

Which MSL version? v4.0.0 or v4.1.0-beta.1 or really 83189ce?

It's 4.0.0. So I guess the problem is still there because the solution was on the MSL master after the 4.0.0 release?

Yes. So it is a matter of updating Dymola, and no longer an issue in this repo. As noted by beutlich the problem occurred when combining all of the sources together (due to how defines and includes worked) - but that was corrected.

beutlich commented 1 week ago

I'd even claim it is not a tool-issue, but a library-issue, i.e. MSL issue: I am actually aware of C/C++ libraries that explicitly check that unity build works and of other C/C++ libraries that explicitly avoid the unity build (all via CMake options).