Skip to content

Conversation

@liu-baoding
Copy link

1. Correct override file generation

Related Issues

Purpose

In ModelicaSystem.py: Fixed a bug where override_variables and simulate_options_override were concatenated without a newline separator. This had caused invalid syntax in the generated override file, where two parameters were merged into a single line, thereby making setSimulationOptions() ineffective.
I found this bug when i trying the example of BouncingBall and changing the stop time of simulation to 2 seconds using mod.setSimulationOptions({"stopTime": "2.0"}), but the change is never effective.
The generated BouncingBall_res_override.txt before fix is like:

e=0.5
g=9.71stopTime=2.0

causes the override using mod.setSimulationOptions({"stopTime": "2.0"}) ineffect.

Approach

A newline separator is added between these two group of override parameters, the generated override file is like:

e=0.5
g=9.71
stopTime=2.0

and the overrided simualte options are working now.

2.Improve OMC message parsing

Related Issues

Purpose

In OMCSession.py, updated the regular expression for parsing ErrorMessage records. The old pattern r"\s*message = \"(.*?)\",\n" of lazy mapping cannot handle correctly escaped double quotes within the message string. Besides, the pattern of id cannot handle the potential negative id. This causes error messages cannot be properly parsed in OMPython.OMCSession.sendExpression.
I found this bug when I execute following code but with some errors occured:

r = mod.optimize()
resultfile_str = r["resultFile"]

This bug results in a TypeError: 'NoneType' object is not subscriptable being thrown (because the result of optimize r is None) rather then the expected OMPython.OMCSession.OMCSessionException.

Approach

The pattern of Regular expression for message and id are corrected, by using greedy mode for message, and adding handling of negative signs for id.

@CLAassistant
Copy link

CLAassistant commented Dec 17, 2025

CLA assistant check
All committers have signed the CLA.

@liu-baoding
Copy link
Author

I investigated the CI failure in tests/test_ModelicaSystem.py::test_getSolutions.

I ran this test locally on Windows and it passed successfully. Since my PR does not modify the core simulate() functionality, and this error time does not end at stopTime appears to be a floating-point precision issue specific to the CI environment (Linux), I believe this failure is unrelated to my changes.

@adeas31
Copy link
Member

adeas31 commented Dec 17, 2025

@syntron can you please check these changes.

@adeas31
Copy link
Member

adeas31 commented Dec 17, 2025

I investigated the CI failure in tests/test_ModelicaSystem.py::test_getSolutions.

I ran this test locally on Windows and it passed successfully. Since my PR does not modify the core simulate() functionality, and this error time does not end at stopTime appears to be a floating-point precision issue specific to the CI environment (Linux), I believe this failure is unrelated to my changes.

Perhaps in the case of empty override an extra newline is added which is causing the simulation to fail.
I restarted the tests. We will see.

@liu-baoding
Copy link
Author

I investigated the CI failure in tests/test_ModelicaSystem.py::test_getSolutions.
I ran this test locally on Windows and it passed successfully. Since my PR does not modify the core simulate() functionality, and this error time does not end at stopTime appears to be a floating-point precision issue specific to the CI environment (Linux), I believe this failure is unrelated to my changes.

Perhaps in the case of empty override an extra newline is added which is causing the simulation to fail. I restarted the tests. We will see.

Thanks for the hint! You were right about the newline issue.

I investigated ModelicaSystem.py and found that when _override_variables or _simulate_options_override are empty, the code was indeed generating an override file with leading empty lines or double newlines.

I've added a .strip() call to the generated content string to ensure the override file has no extra newline. I've pushed this fix in the latest commit. Let's see if this resolves the CI failure.

@adeas31
Copy link
Member

adeas31 commented Dec 17, 2025

It still fails.

Maybe you can run the tests locally. This allows better debugging.

@syntron
Copy link
Contributor

syntron commented Dec 17, 2025

Could this be OM specific?

Just from the latest run the results for tests/test_ModelicaSystem.py::test_getSolutions (only Ubuntu results):

Python 3.10 / stable => OK
Python 3.10 / nightly => fail
Python 3.12 / stable => OK
Python 3.12 / nightly => no data available
Python 3.13 / stable => OK
Python 3.13 / nightly => fail

@adeas31
Copy link
Member

adeas31 commented Dec 18, 2025

#400 should fix the failing tests. Update your PR accordingly once its merged.

Unfortunately I can't merge it yet until we make a new stable release of OM. The new stable release is supposed to have the new runtime flags but we made beta1 without it. So we need to wait until a new release is available.

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.

4 participants