Skip to content
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

Python 3 and PyFMI #146

Closed
dhblum opened this issue Jan 28, 2020 · 14 comments
Closed

Python 3 and PyFMI #146

dhblum opened this issue Jan 28, 2020 · 14 comments
Milestone

Comments

@dhblum
Copy link
Collaborator

dhblum commented Jan 28, 2020

This issue is to update the code to Python 3 and utilize PyFMI without all of JModelica in the test case docker container.

@javiarrobas
Copy link
Contributor

@dhblum for your information. I've performed a quick test here: https://github.com/JavierArroyoBastida/project1-boptest/tree/issue146_python3QuickTest
(see last two commits) where I compile testcase1 with JModelica 2.2. 64bits, Python 2.7 in Windows. Then I switch environment and try to load the previously compiled fmu with pyfmi installed in Python 3 also 64bits version in Windows. If successful, this test would prove that test cases can still be compiled with the latest open-source JModelica release (which runs in Python 2.7), whereas BOPTEST could freely use Python 3 with pyfmi. However, I obtain the following:

Could not find cannot import name 'radau5'
Could not find cannot import name 'dopri5'
Could not find cannot import name 'rodas'
Could not find cannot import name 'odassl'
Could not find ODEPACK functions.
Could not find RADAR5
Could not find GLIMDA.
------------------
sys.version_info:
sys.version_info(major=3, minor=5, micro=5, releaselevel='final', serial=0)
------------------
platform.architecture()[0]:
64bit
------------------
Traceback (most recent call last):
  File "C:\Users\u0110910\workspace\BOPTEST\testcases\testcase1\models\test_load_fmu_from_python3.py", line 29, in <module>
    fmu = load_fmu('wrapped.fmu')
  File "src\pyfmi\fmi.pyx", line 7203, in pyfmi.fmi.load_fmu (src\pyfmi\fmi.c:83983)
  File "src\pyfmi\fmi.pyx", line 6260, in pyfmi.fmi.FMUModelME2.__init__ (src\pyfmi\fmi.c:72216)
  File "src\pyfmi\fmi.pyx", line 3420, in pyfmi.fmi.FMUModelBase2.__init__ (src\pyfmi\fmi.c:40041)
pyfmi.fmi.FMUException: Error loading the binary. Could not load the DLL: A dynamic link library (DLL) initialization routine failed.

Note that the Could not find ... warnings is something that I always get when using pyfmi in Python 3, but that does not hamper the functionality when loading fmus that I compile using Dymola.

@dhblum
Copy link
Collaborator Author

dhblum commented May 26, 2020

@JavierArroyoBastida Thanks for testing. Can you please try compiling with a more recent version of JModelica than 2.2? Preferably 2.10 or the latest trunk?

I've just done the following successfully on Ubuntu 18.04 with a system Python 2.7 installation:

  1. Install latest miniconda for linux with Python 3 (here).
  2. Create an environment called pyfmi and activate it
  3. Install pyfmi using $ conda install -c conda-forge pyfmi (here)
  4. $ python -V gives Python 3.8.2
  5. Deactivate environment and use system Python 2.7 and JModelica (trunk) to compile a test FMU
  6. Activate pyfmi environment and run test script that loads and simulates fmu using pyfmi.

@dhblum
Copy link
Collaborator Author

dhblum commented May 26, 2020

Just an update that I've successfully created an initial docker image containing pyfmi with Python 3 using the steps I outlined above. Worked with a test fmu. I'm working on a better version and testing with BOPTEST integration.

@dhblum
Copy link
Collaborator Author

dhblum commented May 26, 2020

Proposed implementation on https://github.com/ibpsa/project1-boptest/tree/issue146_python3. All unittests pass on that branch locally running examples and testing scripts with Python27.

@dhblum dhblum changed the title Python 3 Python 3 and PyFMI Jun 1, 2020
@dhblum
Copy link
Collaborator Author

dhblum commented Jun 1, 2020

Running unittests with examples and testing scripts pass with Python27 and fail with Python38 with very small numeric differences in two test cases:

From test_testcase1.py
=========================
Of 15 tests run...
Passed: 14
Failures: 1
Errors: 0
Failure Messages
----------------
Failure 0:
Traceback (most recent call last):
  File "/home/travis/build/ibpsa/project1-boptest/testing/test_testcase1.py", line 44, in test_run
    self.compare_ref_timeseries_df(df,ref_filepath)
  File "/home/travis/build/ibpsa/project1-boptest/testing/utilities.py", line 108, in compare_ref_timeseries_df
    self.assertTrue(results['Pass'], '{0} Key is {1}.'.format(results['Message'],key))
AssertionError: False is not true : Max error (0.015410219088890722) in trajectory greater than tolerance (0.001) at index 108. y_test: 138.79913, y_ref:138.81443 Key is PHea_y.
Error Messages
----------------
From test_testcase3.py
=========================
Of 12 tests run...
Passed: 11
Failures: 1
Errors: 0
Failure Messages
----------------
Failure 0:
Traceback (most recent call last):
  File "/home/travis/build/ibpsa/project1-boptest/testing/test_testcase3.py", line 43, in test_run
    self.compare_ref_timeseries_df(df,ref_filepath)
  File "/home/travis/build/ibpsa/project1-boptest/testing/utilities.py", line 108, in compare_ref_timeseries_df
    self.assertTrue(results['Pass'], '{0} Key is {1}.'.format(results['Message'],key))
AssertionError: False is not true : Max error (0.01620968098839968) in trajectory greater than tolerance (0.001) at index 238. y_test: 407.48376, y_ref:407.49993 Key is PHeaNor_y.

A proposal is to drop support for Python27 and update reference results for Python38. Additional testing could be added for Python36, since this is the default Python with Ubuntu 18.04. Note that Python38 is shipped with Ubuntu 20.04.

@dhblum
Copy link
Collaborator Author

dhblum commented Jun 25, 2020

@JavierArroyoBastida Can you test that the new parser implemented at https://github.com/ibpsa/project1-boptest/tree/issue146_python3 works on Windows?

The compile_fmu.py script in the test case repositories should work if run from the testcase models directory per usual. This currently has to be done in a linux environment, or within the JModelica docker container. The proposed changes have the parser start the JModelica docker container and use it to compile the FMU by copying files. In this way, all usage of pymodelica (and therefore Python 2) can be confined to a single script within the parsing directory, called _compile_fmu.py, which gets copied into the docker as well.

[EDIT]: You will need any required external modelica libraries on your MODELICAPATH, such as MBL, IDEAS, or modelica-ibpsa.

@javiarrobas
Copy link
Contributor

@dhblum I've not had the time to review everything in detail (there's a lot to digest), but I've been already testing in Windows and have not succeeded yet to get it working because of commands like export or cp in /testing/makefile which are Linux specific. If you find them strictly necessary a workaround could be using the Cygwin command window instead of PowerShell that I'm using right now. I'll further test next week but just wanted to let you know the state of affairs.

@dhblum
Copy link
Collaborator Author

dhblum commented Jun 30, 2020

Thanks @JavierArroyoBastida for checking. I understand that the unit tests will only work in Linux because of that command. I am more concerned with checking that the new parser (parsing/parser.py) works on Windows (it uses Python 3 and system commands to interact with the JModelica docker image).

This was referenced Jul 2, 2020
@javiarrobas
Copy link
Contributor

I'm proposing a few changes here. The only necessary edit to get it running in Windows is the one regarding the separator of environmental variables: 36d5a68, which was hard coded before.

I got to the point of compiling a model by calling the compile_fmu.py of testcase1 from python3 in my local host. This call successfully creates the model fmu in the local directory (_compile_fmu successfully creates an fmu in the jm container, copies it out of the container, and returns the right path to the fmu in the local host). However, I'm afraid there's still a major issue to address with this approach: the generated fmu is compiled with Linux binaries, whereas it still needs to be loaded in Windows to parse the instances. Therefore, in the following line:
https://github.com/JavierArroyoBastida/project1-boptest/blob/e6583a0b37ad486be782c7dabe9db79343d54646/parsing/parser.py#L50
I get the exception:
pyfmi.fmi.FMUException: Error loading the binary. The FMU contains no binary for this platform.

It does not prevent from passing the tests since these run in a Linux environment (tests pass) but does not allow to compile test cases in Windows.

@dhblum
Copy link
Collaborator Author

dhblum commented Jul 2, 2020

Thanks a lot for the review and testing. The point about parsing on Windows is well-taken. One thing to consider though is that the BOPTEST container environment is linux, so the resulting FMU needs to have linux binaries in the end. But there are a few options if we want test cases to be able to be compiled in Windows anyway:

  1. Continue to require the parser to utilize Python 2 and be used in an environment that has JModelica installed.
  2. Use a text-based parsing method, perhaps with modelica-json (https://github.com/lbl-srg/modelica-json), to parse the .mo file(s) directly and then compile the wrapped.mo once into wrapped.fmu. There could be a flag to indicate whether to compile in a docker container linux environment (using the jm docker image) or the native host environment.

@javiarrobas
Copy link
Contributor

I think I like more the approach of using the modelica-json parser (if the parser is as effective and reliable as pyfmi). That would allow to maintain your latter edits towards switching to Python 3 as they are, and maybe we could even avoid one of the two compilations in the export_fmu call? (first compilation is only used to parse instances if I'm not mistaken).
I've no experience at all with the modelica-json parser though, so I've no idea of the amount of work that that would require... If it's too much maybe we should stick to requiring the parser to utilize Python 2.

@dhblum
Copy link
Collaborator Author

dhblum commented Aug 6, 2020

I think right now the cleanest approach could be to require Python 2 for use of the parser, but use Python 3 everywhere else. I will propose a PR with such an implementation. Then, as a separate issue, we can think about upgrading the parser to utilize modelica-json or other approach.

@dhblum
Copy link
Collaborator Author

dhblum commented Aug 25, 2020

Simulating some, generally more complex, FMUs as ME with pyfmi installed via conda forge result in the following error. This was produced with Buildings.Examples.VAVReheat.ASHRAE2006:

Traceback (most recent call last):
  File "simulate_fmu.py", line 5, in <module>
    model.simulate(0,3600, options=options)
  File "src/pyfmi/fmi.pyx", line 7733, in pyfmi.fmi.FMUModelME2.simulate
  File "src/pyfmi/fmi.pyx", line 313, in pyfmi.fmi.ModelBase._exec_simulate_algorithm
  File "src/pyfmi/fmi.pyx", line 309, in pyfmi.fmi.ModelBase._exec_simulate_algorithm
  File "/home/dhbubu18/miniconda3/envs/boptest/lib/python3.8/site-packages/pyfmi/fmi_algorithm_drivers.py", line 549, in solve
    self.simulator.simulate(self.final_time, self.ncp)
  File "assimulo/ode.pyx", line 180, in assimulo.ode.ODE.simulate
  File "assimulo/ode.pyx", line 300, in assimulo.ode.ODE.simulate
  File "assimulo/explicit_ode.pyx", line 101, in assimulo.explicit_ode.Explicit_ODE._simulate
  File "assimulo/explicit_ode.pyx", line 188, in assimulo.explicit_ode.Explicit_ODE._simulate
  File "assimulo/solvers/sundials.pyx", line 1935, in assimulo.solvers.sundials.CVode.integrate
  File "assimulo/solvers/sundials.pyx", line 1950, in assimulo.solvers.sundials.CVode.integrate
  File "assimulo/solvers/sundials.pyx", line 2194, in assimulo.solvers.sundials.CVode.initialize_options
assimulo.exception.AssimuloException: No support for SuperLU was detected, please verify that SuperLU and SUNDIALS has been installed correctly.

This issue should be resolved before merging the approach to master.

@dhblum
Copy link
Collaborator Author

dhblum commented Oct 3, 2023

Closed by #536.

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

No branches or pull requests

2 participants