-
Notifications
You must be signed in to change notification settings - Fork 70
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
Comments
@dhblum for your information. I've performed a quick test here: https://github.com/JavierArroyoBastida/project1-boptest/tree/issue146_python3QuickTest
Note that the |
@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:
|
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. |
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. |
Running unittests with examples and testing scripts pass with Python27 and fail with Python38 with very small numeric differences in two test cases:
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. |
@JavierArroyoBastida Can you test that the new parser implemented at https://github.com/ibpsa/project1-boptest/tree/issue146_python3 works on Windows? The [EDIT]: You will need any required external modelica libraries on your MODELICAPATH, such as MBL, IDEAS, or modelica-ibpsa. |
@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 |
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 ( |
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 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. |
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:
|
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 |
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. |
Simulating some, generally more complex, FMUs as ME with pyfmi installed via conda forge result in the following error. This was produced with
This issue should be resolved before merging the approach to master. |
Closed by #536. |
This issue is to update the code to Python 3 and utilize PyFMI without all of JModelica in the test case docker container.
The text was updated successfully, but these errors were encountered: