-
Notifications
You must be signed in to change notification settings - Fork 230
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
fix juliacall
implementation
#2754
base: feat/py39_rebase
Are you sure you want to change the base?
Conversation
- CI updates: run CI on all platforms with and without RMS, split regression tests into separate job after that so that regression is only checked on one platform - installation updates: make RMG-Py actually `pip`-installable as `reactionmechanismgenerator` to avoid having to set the `PYTHONPATH` variable, which breaks `juliacall`, add a convenience script for installing RMS - code changes: fix some small bugs in the new optional-rms setup (one missed `requires_rms` and one incorrect rebase)
6d03db1
to
0e33bf3
Compare
Hmm...
|
@jonwzheng there is one final small issue - the above regression test errors out. Can you take a look? |
looking into it now, just going to type out what I see so far:
The model size is identical to the (currently passing) main branch test: https://github.com/ReactionMechanismGenerator/RMG-Py/actions/runs/12984841799/job/36208494954#step:12:20982 which indicates that the model generation part is fine. Not likely an issue with julia calls are handled in Proximal cause:
|
I think this is the problem: main...feat/py39_rebase#diff-e3891a3f45a445b7ce7074350473a30915530d790d1c56025a10195281742948R555
Pushing a patch that will fix this in a minute. |
…nstantTLiquidSurfaceReactor
Okay, so changing
The surface initial conditions contains I'm kind of confused why the code was working at all before, because it seems like this issue would've been encountered even in the old code. |
Met in person to talk about this. the branch of RMS used in this PR (the one that contains the new JuliaCall code) was not up to date with the We have rebased our branch onto that one, and restarted the CI. |
Regression Testing ResultsWARNING:root:Initial mole fractions do not sum to one; normalizing. Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:08
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
aromatics Passed Core Comparison ✅Original model has 15 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
aromatics Passed Edge Comparison ✅Original model has 106 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
Observables Test Case: Aromatics Comparison
✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! aromatics Passed Observable Testing ✅Regression test liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:02:22
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
liquid_oxidation Passed Core Comparison ✅Original model has 37 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
liquid_oxidation Failed Edge Comparison ❌Original model has 214 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
Observables Test Case: liquid_oxidation Comparison
✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! liquid_oxidation Passed Observable Testing ✅Regression test nitrogen:Reference: Execution time (DD:HH:MM:SS): 00:00:01:27
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
nitrogen Passed Core Comparison ✅Original model has 41 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
nitrogen Failed Edge Comparison ❌Original model has 133 species. Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(O2s-CdN3d) + group(N3d-OCd) + group(Cd-HN3dO) + ring(oxirene) + radical(CdJ-NdO) Non-identical kinetics! ❌
kinetics:
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
Observables Test Case: NC Comparison
✅ All Observables varied by less than 0.200 on average between old model and new model in all conditions! nitrogen Passed Observable Testing ✅Regression test oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:02:27
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
oxidation Passed Core Comparison ✅Original model has 59 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
oxidation Passed Edge Comparison ✅Original model has 230 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
Observables Test Case: Oxidation Comparison
✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! oxidation Passed Observable Testing ✅Regression test sulfur:Reference: Execution time (DD:HH:MM:SS): 00:00:00:55
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
sulfur Passed Core Comparison ✅Original model has 27 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
sulfur Failed Edge Comparison ❌Original model has 89 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
Observables Test Case: SO2 Comparison
The following observables did not match: ❌ Observable species O=S=O varied by more than 0.100 on average between old model SO2(15) and new model SO2(15) in condition 1.
sulfur Failed Observable Testing ❌Regression test superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:00:35
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
superminimal Passed Core Comparison ✅Original model has 13 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
superminimal Failed Edge Comparison ❌Original model has 18 species. Non-identical thermo! ❌
Identical thermo comments: Regression test RMS_constantVIdealGasReactor_superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:02:24
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅Original model has 13 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅Original model has 13 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
Observables Test Case: RMS_constantVIdealGasReactor_superminimal Comparison
✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! RMS_constantVIdealGasReactor_superminimal Passed Observable Testing ✅Regression test RMS_CSTR_liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:06:14
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
RMS_CSTR_liquid_oxidation Passed Core Comparison ✅Original model has 37 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
RMS_CSTR_liquid_oxidation Passed Edge Comparison ✅Original model has 248 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
Observables Test Case: RMS_CSTR_liquid_oxidation Comparison
✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! RMS_CSTR_liquid_oxidation Passed Observable Testing ✅Regression test fragment:Reference: Execution time (DD:HH:MM:SS): 00:00:00:42
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
fragment Passed Core Comparison ✅Original model has 10 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
fragment Passed Edge Comparison ✅Original model has 33 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
Observables Test Case: fragment Comparison
✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! fragment Passed Observable Testing ✅Regression test RMS_constantVIdealGasReactor_fragment:Reference: Execution time (DD:HH:MM:SS): 00:00:03:04
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
RMS_constantVIdealGasReactor_fragment Passed Core Comparison ✅Original model has 10 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
RMS_constantVIdealGasReactor_fragment Passed Edge Comparison ✅Original model has 27 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
Observables Test Case: RMS_constantVIdealGasReactor_fragment Comparison
✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! RMS_constantVIdealGasReactor_fragment Passed Observable Testing ✅Regression test minimal_surface:Reference: Execution time (DD:HH:MM:SS): 00:00:00:44
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
minimal_surface Passed Core Comparison ✅Original model has 11 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
minimal_surface Passed Edge Comparison ✅Original model has 38 species.
Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython
Observables Test Case: minimal_surface Comparison
✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! minimal_surface Passed Observable Testing ✅beep boop this comment was written by a bot 🤖 |
pip
-installable asreactionmechanismgenerator
to avoid having to set thePYTHONPATH
variable, which breaksjuliacall
, add a convenience script for installing RMSrequires_rms
and one incorrect rebase)This PR replaces #2749