OMPython icon indicating copy to clipboard operation
OMPython copied to clipboard

Fix: Correct override file generation and improve OMC message parsing

Open liu-baoding opened this issue 2 months ago • 9 comments

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.

liu-baoding avatar Dec 17 '25 06:12 liu-baoding

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Dec 17 '25 06:12 CLAassistant

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.

liu-baoding avatar Dec 17 '25 09:12 liu-baoding

@syntron can you please check these changes.

adeas31 avatar Dec 17 '25 11:12 adeas31

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.

adeas31 avatar Dec 17 '25 11:12 adeas31

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.

liu-baoding avatar Dec 17 '25 13:12 liu-baoding

It still fails.

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

adeas31 avatar Dec 17 '25 17:12 adeas31

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

syntron avatar Dec 17 '25 19:12 syntron

#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.

adeas31 avatar Dec 18 '25 11:12 adeas31

@liu-baoding please check if PR #400 and PR #402 fixes the override bug as this part of the code was completly rewritten

the improvement of OMC message parsing is still needed; if the override bug is fixed, this PR could be used

syntron avatar Dec 29 '25 14:12 syntron

@liu-baoding both #400 and #402 are merged. You can rebase this PR with only the message parsing change.

adeas31 avatar Jan 26 '26 14:01 adeas31