-
Notifications
You must be signed in to change notification settings - Fork 112
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
Support arm64 architecture on macOS #532
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #532 +/- ##
=======================================
- Coverage 98.8% 98.7% -0.1%
=======================================
Files 44 44
Lines 4804 4803 -1
=======================================
- Hits 4747 4744 -3
- Misses 57 59 +2
|
97d9f30
to
1d40c22
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #532 +/- ##
=====================================
Coverage 98.8% 98.8%
=====================================
Files 44 44
Lines 4796 4813 +17
=====================================
+ Hits 4739 4756 +17
Misses 57 57
|
788ce9b
to
eb15a0e
Compare
@glatterf42 let me know whether you think there is urgency to include this in the 3.9 release. We would need to invite and wait for reviews from:
Given this might take a few days, and the first three are probably capable to use it from If we wait, then I will also wait until after the workshop to request a review of iiasa/actions#13. |
I'd also say it's likely not urgent. Workshop users will learn about GitHub anyway and if they need to use this branch, this will be a good opportunity to practice. They could also then provide feedback about their install process. |
@ravitby @SiddharthJoshi-Git @setupelz —could you please try the steps above under "How to review" / "message_ix usage" and let us know the results? @phackstock —could you please comment on the questions above under "How to review" / "Scenario Explorer usage"? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pinging me @khaeru. As far as I know there's no GAMS installation for the Scenario Explorer.
Will the absence of the .dylib, .so, and/or .dll files bundled with the ixmp Python package affect these deployments?
I also don't think that that would be an issue.
Green light from my side to merge. If there are any issues that would arise from the Scenario Explorer side, we can always pin a previous version and open an issue here.
Great, thanks for confirming. |
3678488
to
1176fd2
Compare
@glatterf42, since the other requested colleagues seem unable to review this over the past ~6 weeks, I propose we merge and wait for feedback that may come as they/others use it from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me, just one possible typo :)
Include review suggestion by @glatterf42: #532 (review)
- Use tempfile.TemporaryDirectory. - Log warning if gams is missing. - Create maximum one instance. - Sort globals, classes, and functions in .model.gams
- Use macos-13 for Python 3.8–3.9. - Use macos-latest for Python 3.10–3.12.
This PR responds to #531.
.backend.jdbc
:Pass
-Djava.library.path=…
when starting the JVM, giving two directories containing the required binary libraries (with the extension.so
on Linux;.dll
on Windows; and.dylib
on macOS):The second directory is redundant if the first one is present (i.e. if a GAMS installation exists). To confirm this, the PR currently deletes the bundled libraries entirely. These commits could also be dropped instead.
How to review
message_ix
usage: Installixmp
from this branch on a macOS arm64 system with an arm64 version of Python. Run the test suite or other code and confirm the code functions as expected.message_ix
deployments; particularly, for the Scenario Explorer. Is GAMS installed in these contexts? Will the absence of the .dylib, .so, and/or .dll files bundled with the ixmp Python package affect these deployments?PR checklist
Add, expand, or update documentation.