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

Upgrade OpenModelica #641

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

Upgrade OpenModelica #641

wants to merge 8 commits into from

Conversation

vtnate
Copy link
Contributor

@vtnate vtnate commented Jul 2, 2024

Any background context you want to provide?

A new version of OpenModelica was recently released. We can now take advantage of their "many improvements and bug fixes" in this newest version.

What does this PR accomplish?

  • Update Dockerfile to use new OM version
    • Rename a variable in the Dockerfile
    • Move cmake to a different part of the Dockerfile to be as similar as possible to the OM Dockerfile ours is modeled on
  • Declare new version of the Dockerfile in the Modelica readme
  • Refer to the not-yet-created/uploaded Docker image in modelica_runner.py

How should this be manually tested?

  • Generate new Docker image according to the docs
    • Tag it 2.1.03.0.0 (@nllong is that the right version for this change?)
  • Upload to NREL's dockerhub account
  • Run test suite
    • These tests are the ones that work with Ubuntu 20 but not with 22 or 24 😢 (due to something in this file that LBNL isn't interested in addressing) and make a good canary in the coal mine.

What are the relevant tickets?

Screenshots (if appropriate)

@vtnate vtnate added the dependencies Pull requests that update a dependency file label Jul 2, 2024
@vtnate vtnate requested a review from nllong July 2, 2024 15:06
@vtnate vtnate self-assigned this Jul 2, 2024
@nllong
Copy link
Member

nllong commented Jul 3, 2024

tests failing? should I wait until that gets resolved to review?

@vtnate
Copy link
Contributor Author

vtnate commented Jul 3, 2024

tests failing? should I wait until that gets resolved to review?

No, please review now. The tests are failing because in modelica_runner.py I'm referring to an image that doesn't exist yet.

Will you please:

  1. confirm that version 2.1.0 is correct
  2. build the image
  3. run the tests locally (if you don't trust me 😆)
  4. push the image to dockerhub

Once those are good, I can re-run CI and once we have all green you can approve and we can merge it.

@vtnate vtnate changed the title Bump OM to 1.23.0 Upgrade OpenModelica Jul 5, 2024
@vtnate vtnate marked this pull request as draft October 8, 2024 21:38
@vtnate vtnate marked this pull request as ready for review October 9, 2024 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants