-
Notifications
You must be signed in to change notification settings - Fork 169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bump used Modelica language version to 3.6 for MapleSim moparser (CI) #4235
Conversation
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. |
@beutlich : I guess @maltelenz is better suited to review this PR. I was not present at the last MAP-Lib meeting. |
@hubertus65 recorded the following in the chat on Teams for MAP-LIB Monthly: |
@casella, this PR is all ready to merge, but seems to be failing at checks. Could you please look into it? |
96af39f
to
3238c8e
Compare
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. |
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. |
MAP-LIB meeting 2024-02-02: |
@Harisankar-Allimangalath Please back-port to release-branch v4.1.0 |
Sorry, I cannot follow. Enabling MoLang 3.6 syntax also opens all MoLang 3.6 features (as the new break keyword). |
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. |
@maltelenz @beutlich How should this be proceeded ? |
@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? |
Because we do not want to allow new syntax features in the branched-off maint branch, do we? |
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 |
@AHaumer should I backport this? 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 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: |
Uff, not sure if this can be tackled at all while also updating the simulation tool.
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 :( |
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. |
According to #4175 this should not be merged right now.