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.9 #2741

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Python 3.9 #2741

wants to merge 16 commits into from

Conversation

JacksonBurns
Copy link
Contributor

@JacksonBurns JacksonBurns commented Dec 3, 2024

Upgrading RMG-Py to Python 3.9

The current version of Python used by RMG (3.7) is deprecated, so we are missing out on the latest updates to our various dependencies. This PR upgrade RMG to use Python 3.9 by doing the following:

Getting this merged

One feature that remains to be implemented and will have to be figured out in a separate PR into this one is the new juliaup + juliacall setup implemented by @hwpang. It will be much better, but there are currently some bugs preventing it from working (at all), primarily related to the installed julia executable being able to 'see' the Python environment and its installed dependencies.

After that, this should be good to merge. RMG-Py will be appreciably faster, easier to extend, and less buggy.

@JacksonBurns
Copy link
Contributor Author

JacksonBurns commented Dec 6, 2024

@jonwzheng @donerancl the current unit test failure seems to indicate that benzene is not being perceived as aromatic, preventing it from being matched in the tree (maybe?) aromatic perception was correct, see below

@JacksonBurns
Copy link
Contributor Author

JacksonBurns commented Dec 6, 2024

Need to check (1) if resonance structure generation happens during the failing test and (2) does the new implementation work on benzene, with and without the bonds already being of order 1.5 (aromatic)? confirmed that the updated resonance generation is correct for benzene

hwpang and others added 14 commits January 14, 2025 11:49
These squashed commits contain all of the initial efforts by HWP to switch from pythoncall to JuliaCall
These were the first commits to change to Python 3.9, and they do not work on their own but instead open the door to finding issues to make the rest of the changes
here are all the solutions that were implemented:
 - in a couple places we were implicitly casting a double to a numpy float, now need to explicitly grad the _real_ part of the double
 - Cython does not allow nested function declaration inside cpdef functions, move all of these outside of their parent functions or redefine them to be purely functional where practical
 - similar to the above, lambda function are no longer allowed - get the same treatment. what's different here is that usually we are using lambda in sorting calls, so we can replace these with operator.itemgetter or attrgetter, where relevant. this also involves re-writing a couple reduce calls, which also used lambda
 - modern cython does not allow re-declaring existing Cython variables (in previous versions this was a warning I think), so I just remove these where needed. (btw Cython is super cool, actually points out both of the declaration so that you can delete one)

i made these fixes while listening to The Metal by Tenacious D, Talk Too Much by Renee Rapp, BEST INTEREST by Tyler, The Creator (love the bass line), mos thoser by food house, and Doritos & Fritos by 100 Gecs
These commits are the second (and final) round of attempting JuliaCall debugging by HWP
These commits reflect the initial effort by XD to re-implement the MILP for Clar optimization in SciPy. They were unsuccessful because at the time RMG could not actually run a required version of Python to install SciPy with MILP.
These commits rescued some abandoned work from another branch
This is a collection of largely unordered commits which implement all of the changes discussed in the commit name
@JacksonBurns
Copy link
Contributor Author

0a13902 fixed the single remaining failing unit test. Rebased on the latest version on main, will now let the tests run here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants