-
Notifications
You must be signed in to change notification settings - Fork 228
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.9] Overhaul RMG-RMS Python-Julia dependencies (juliaup
and pythoncall
), Add Tests for Arkane Network Algorithms
#2687
base: main
Are you sure you want to change the base?
Conversation
I'll update this comment with the path forward for each platform as the errors roll in. All PlatformsThere are Cython errors all over the place, namely because we have incremented Cython to: M1 MacsThe following packages are outright missing, but without any conflicts:
Anything in the RMG channel we can build for M1 macs, if it comes to that. UbuntuWe will need to rebuild the Intel MacsSame as Ubuntu. |
Julia and Pyjulia would be taken care of by #2640 (which would be unblocked if we could get past Python 3.7), so that leave muq? (and the ones that are our responsibility) |
Is this faring any better than 3.10 or 3.11? at least it solved the boost - rdkit - muq issue? or do we need to await results on non-M1-platforms? |
Local testing shows that this resolves the muq + rdkit issue. I tried leaving in our packages ( |
@rwest initial tests are done. M1 macs are missing a number of packages that are outside of our control. I think that is a dead end. Ubuntu and MacOS need the All platforms will need Cython errors fixed. |
Following some offline discussion, @hwpang and I have agreed that I will rebase this PR onto #2640 and then merge it into the same. This will:
|
I was able to make the MacOS to not install rmgmolecule. However, now we run into a new error:
|
Ok so interesting developments - the Linux build continues to fail when calling out to Lpsolve but the Intel Mac (which didn't use to even install RMS at all, so this is surprising) passes that test, later failing during this unit test because of something I did with Cython, I think:
The M1 error... no idea. Our current docs tell users to just have the M1 hardware emulate the old arch so that they can run old arch binaries - perhaps we do the same in the CI? |
Yeah that sounds fine to me. |
for some reason `functools.partial` is allowed, but not `lambda`
@hwpang ARM mac is now installing Intel packages, but this error pops up during RMS installation:
|
Stack overflow says it's because we are setting PYTHONPATH https://stackoverflow.com/a/65740883 |
Thanks. The only PYTHONPATH it has is the PYTHONPATH we set for the RMG-Py, and we didn't set any other PYTHONPATH. Any ideas on what we could try?
|
So unsetting the PYTHONPATH didn't solve the problem we encountered with ARM mac. It failed with the same error:
It has also caused the previously working Ubuntu installation to fail. So I'm gonna drop that commit. Would appreciate any ideas to try. @JacksonBurns |
This looks promising, right? What @hwpang pushed yesterday compiled and installed everything and started to run all the unit tests? Update: it could feasibly be the underlying |
Apologies for the delay, travel and also @hwpang is Dr. @hwpang now 🎉! I am not sure what else to try RE: ARM Mac is busted. Running ARM Packages natively failed at the Julia installation, emulating Intel packages failed during Julia installation also but for a different, possibly Intel Mac is broken on GitHub because it is too slow to install RMS (except when sometimes it does, for some reason, only to fail due to a Cython error I think I caused). Ubuntu is failing because lpsolve crashes the whole test run, and frustratingly refuses to even give a helpful trace. For now, maybe we try and figure out why lpsolve is failing? |
How hard is it to remove the need for LPSolve?.... #2623 and #2596 (comment) |
@rwest This is sort of a chicken and egg problem, because replacing lpsolve with scipy.optimize.milp requires python 3.9, and this PR that upgrades RMG to python 3.9 is failing due to lpsolve :( |
An alternative thought, maybe we can rebase #2623 on this PR and see if that resolves the lpsolve issue? |
Worth a try. We have both the chickens and the eggs as open PRs. We should try putting them together. |
I will (1) make a copy of @xiaoruiDong's branch in #2623 and push it to ReactionMechanismGenerator/RMG-Py and then (2) rebase that branch on this branch and then (3) open a PR from that branch into this one |
@hwpang's #2596 is also required in combination with #2694 in order to completely remove lpsolve. I will (1) make a copy of that branch on RMG-Py (2) rebase it onto #2694 (3) tack on a commit to remove lpsolve (4) open a PR into #2694 Merge order would then be Isodesmic into CLAR, CLAR into this, and this into main. |
juliaup
and pythoncall
), Add Tests for Arkane Network Algorithms, Upgrade to Python 3.9juliaup
and pythoncall
), Add Tests for Arkane Network Algorithms
I lost track of the other PRs. Is this idea of removing lpsolve looking promising? |
Don't fix merge conflicts in this PR, I have already done so in #2694 |
Replace #2635, targeting a smaller improvement in Python minor version to keep better compatibility with our dependencies.