ReactionMechanismSimulator.jl
ReactionMechanismSimulator.jl copied to clipboard
Overhaul RMG-RMS Python-Julia dependencies
@mjohnson541 pointed out that this will likely break RMG's CI. Could you point out where in RMG CI do we need to change to avoid breaking it? cc @JacksonBurns
@mjohnson541 Recording our offline discussion here: We should replace pyjulia with JuliaCall in RMG (see here: https://juliapy.github.io/PythonCall.jl/stable/pycall/). It should make the installation process easier. cc @JacksonBurns
Codecov Report
Attention: Patch coverage is 51.59574%
with 91 lines
in your changes missing coverage. Please review.
Project coverage is 48.42%. Comparing base (
1f37bfd
) to head (6ce8bfa
).
:exclamation: Current head 6ce8bfa differs from pull request most recent head 827a1a4
Please upload reports for the commit 827a1a4 to get more accurate results.
Files | Patch % | Lines |
---|---|---|
src/PhaseState.jl | 61.76% | 52 Missing :warning: |
src/ReactionMechanismSimulator.jl | 0.00% | 27 Missing :warning: |
src/Reactor.jl | 44.44% | 10 Missing :warning: |
src/Plotting.jl | 0.00% | 1 Missing :warning: |
src/fluxdiagrams.jl | 0.00% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #256 +/- ##
==========================================
- Coverage 48.71% 48.42% -0.30%
==========================================
Files 31 31
Lines 8313 8351 +38
==========================================
- Hits 4050 4044 -6
- Misses 4263 4307 +44
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@hwpang you can open a pull request against RMG-Py and change this line of the CI file (https://github.com/ReactionMechanismGenerator/RMG-Py/blob/eed950a389738e70894c25e1d8388bb20ef75947/.github/workflows/CI.yml#L155) to install this branch of RMS from your repository. We can then just see the failures live.
RMS-RMG twin PR: https://github.com/ReactionMechanismGenerator/RMG-Py/pull/2640
I've done some verification on the juliacall end:
- The primary trick I used to make pyrms from pyjulia works with juliacall
- I think we can get around the need for diffeqpy, at least I think I circumvented our original reason for needing it...which should be nice because pyrms will no longer need the full DifferentialEquations.jl installed.
I think in terms of merge order this would look like: merge the RMS PR, merge a pyrms PR, build new binaries for pyrms, and then merge RMG PR.
@mjohnson541 Can you make the pyrms PR since you've looked into this? Thanks
@mjohnson541 Can you make the pyrms PR since you've looked into this? Thanks
Yes, I'm waiting until this PR is building properly, right now I wouldn't be able to test the pyrms PR properly.
@mjohnson541 I addressed your comment and had a question. I will write pretty commit messages after we agree on the changes
Removing the duplicate export would cause the test to fail. I undo the change.
@mjohnson541 This is ready for another review / approve. Need to clean up the commits after approval and coordinate to merge at the same time with https://github.com/ReactionMechanismGenerator/RMG-Py/pull/2640.