Bump used Modelica language version to 3.6 for MapleSim moparser (CI)

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

472 stars 169 forks source link

Bump used Modelica language version to 3.6 for MapleSim moparser (CI) #4235

Closed beutlich closed 8 months ago

beutlich commented 10 months ago

According to #4175 this should not be merged right now.

maltelenz commented 10 months ago

There was a decision in the monthly MAP-Lib meeting on the 14th of November, to switch to 3.6 fully, and consider for each PR that tries to use 3.6 features, whether we are ready to accept it for 4.1.0 or not.

Unfortunately it doesn't seem this was written down anywhere. It was discussed in the context of #4208.

So this should be possible to merge.

tobolar commented 10 months ago

@beutlich : I guess @maltelenz is better suited to review this PR. I was not present at the last MAP-Lib meeting.

eshmoylova commented 10 months ago

@hubertus65 recorded the following in the chat on Teams for MAP-LIB Monthly:

If we say in the documentation that we support 3.6 then we need to support that claim elsewhere.

henrikt-ma commented 9 months ago

@casella, I suppose you as the project leader is the right person to push the merge button on this.

Edit: My bad, I was confused and mixed up this PR with https://github.com/modelica/ModelicaStandardLibrary/pull/4208.

arunkumar-narasimhan commented 8 months ago

@casella, this PR is all ready to merge, but seems to be failing at checks. Could you please look into it?

beutlich commented 8 months ago

but seems to be failing at checks.

Not any more.

Note, that merging this PR opens the door for PRs like #4231 which I'd not to have right now. Therefore I'd not like to have it merged now.

henrikt-ma commented 8 months ago

Note, that merging this PR opens the door for PRs like #4231 which I'd not to have right now. Therefore I'd not like to have it merged now.

This sounds like a misunderstanding to me. The door for #4231 is already open version-wise as of #4208. What is needed in order to move forward with #4231 shouldn't have anything to do with the MapleSim moparser, but what we need is an agreement with tool vendors that they welcome the adoption of the specific language feature selective model extension.

Hence, I believe this PR is fine. Moving it out of Draft state would seem like a good start.

GallLeo commented 8 months ago

MAP-LIB meeting 2024-02-02: We already agreed to not open up to all Modelica 3.6 features. But, we bumped the version in order to e.g. use figure annotations (as long as no tool vendor complains). Therefore, syntax check should also be bumped to Modelica 3.6.

GallLeo commented 8 months ago

@Harisankar-Allimangalath Please back-port to release-branch v4.1.0

beutlich commented 8 months ago

MAP-LIB meeting 2024-02-02: We already agreed to not open up to all Modelica 3.6 features. But, we bumped the version in order to e.g. use figure annotations (as long as no tool vendor complains). Therefore, syntax check should also be bumped to Modelica 3.6.

Sorry, I cannot follow. Enabling MoLang 3.6 syntax also opens all MoLang 3.6 features (as the new break keyword).

maltelenz commented 8 months ago

That was what was decided last year, that we would open up for 3.6 generally. However, for each individual pull request that uses new features, tool vendors can object that they are not ready.

Harisankar-Allimangalath commented 8 months ago

@maltelenz @beutlich How should this be proceeded ?

maltelenz commented 8 months ago

@Harisankar-Allimangalath please create a pull request to backport to 4.1.0 maint branch.

beutlich commented 8 months ago

@Harisankar-Allimangalath please create a pull request to backport to 4.1.0 maint branch.

Really? I hope that we do not do this.

maltelenz commented 8 months ago

@Harisankar-Allimangalath please create a pull request to backport to 4.1.0 maint branch.

Really? I hope that we do not do this.

Could you explain why not?

beutlich commented 8 months ago

Because we do not want to allow new syntax features in the branched-off maint branch, do we?

maltelenz commented 8 months ago

The decision by MAP-LIB was to use Modelica 3.6 in 4.1.0, so yes, technically we do want to allow exactly that.

In practice, tool vendors will object if a pull request using break appears on the maint branch for 4.1.0.

Harisankar-Allimangalath commented 8 months ago

@AHaumer should I backport this? This delays the tool vendor testing.

beutlich commented 8 months ago

This delays the tool vendor testing.

I am afraid I cannot follow. This a CI feature and tool vendors can test with or w/o it already.

Harisankar-Allimangalath commented 8 months ago

@beutlich ok , so once #4299 and #4298 is merged I can tagout the beta tag right ?

maltelenz commented 8 months ago

@Harisankar-Allimangalath The real blocker for tool vendor testing (at least ours) is that we don't have new reference results.

beutlich commented 8 months ago

@beutlich ok , so once #4299 and #4298 is merged I can tagout the beta tag right ?

IMO it is not yet there. The changes since alpha.1 are hardly relevant so far whereas the actual open issues haven't be addressed yet.

beutlich commented 7 months ago

The real blocker for tool vendor testing (at least ours) is that we don't have new reference results.

Unblocking -> Results by Dymola 2024x of MSL v4.1.0-beta.1 (with serveral necessary changes) are here.

maltelenz commented 7 months ago

The real blocker for tool vendor testing (at least ours) is that we don't have new reference results.

Unblocking -> Results by Dymola 2024x of MSL v4.1.0-beta.1 (with serveral necessary changes) are here.

While the effort is appreciated, I was hoping for these things before looking at correctness failures for System Modeler:

Only updating the reference results for models where we have good reason (model changes)
At least a categorization of the current failures in the regression testing with Dymola, which I think @GallLeo and @casella started on?

beutlich commented 7 months ago

While the effort is appreciated, I was hoping for these things before looking at correctness failures for System Modeler:
* Only updating the reference results for models where we have good reason (model changes)

Uff, not sure if this can be tackled at all while also updating the simulation tool.

* At least a categorization of the current failures in the regression testing with Dymola, which I think @GallLeo and @casella started on?

Well, #4296 only touched four models and there was no vibrant discussion going on which I would have expected before releasing a beta release.

maltelenz commented 7 months ago

Well, #4296 only touched four models and there was no vibrant discussion going on which I would have expected before releasing a beta release.

Right. I don't know what the intention is with the beta, but if it is a signal that it is ready for tool vendor testing, it's clearly not the case yet :(

beutlich commented 7 months ago

Yes, intention of the beta is to initiate tool vendor testing. However with reference results being avialble only now, the process is already broken. Still, you should not be blocked anymore.