Fake PR to test CI regression fix: this branch should fail regression tests#2803
Closed
sevyharris wants to merge 11 commits intoReactionMechanismGenerator:mainfrom
Closed
Fake PR to test CI regression fix: this branch should fail regression tests#2803sevyharris wants to merge 11 commits intoReactionMechanismGenerator:mainfrom
sevyharris wants to merge 11 commits intoReactionMechanismGenerator:mainfrom
Conversation
Commit 60c4125 recently tweaked this and introduced some errors. The `&> checkModels.log` syntax overwrites the checkModels.log file every time, with the combination of stderr and stdout, rather than appending to it. Thus some of the intended output of the observables testing would be captured, and then only the last iteration of it would be reported during the "Debugging info" block. Also, the use of back ticks in the bash echo "<summary>...`checkModels.py`.." line actually executed the checkModels.py script rather than just printing its name! But rather than fix this, it is now removed, since it is no longer used. The current implementation, hopefully, hides the errors in a summary/details block, but reports them closer to where they occur. In general, we should not be hiding or throwing away warnings and error messages!
There was a note about what we could do once we upgrade Python, which we now have.
You can either import like this import rmgpy rmgpy.DISABLE_JULIA = True import rmgpy.rmg.reactionmechanismsimulator_reactors or you can set the environment variable like RMG_DISABLE_JULIA=1 before loading the python script that imports it. Either way, it'll skip importing the juliacall stuff, saving compile time, and not logging a warning to the stderr.
They're not needed. Trying (and failing) to import them adds warnings to the stderr stream which ends up in the regression CI logs.
If these examples don't use RMS reactors, they can set the RMG_DISABLE_JULIA environment variable when calling RMG to disable it and speed things up.
make eg8 make eg9 make eg10 Now run examples with RMS reactors and Julia, should you want to test those.
This is to avoid the line WARNING:root:Julia import failed, RMS reactors not available. Exception: No module named 'juliacall' Stacktrace: showing up in the regression results. But it might speed things up a little too.
Just to save wall clock time, we could let the regression tests start before the full matrix of unit tests is complete. Some of them (especially the mac os ones) take a couple of hours. Possible drawback is we'll get comment annotations about the regression tests even if the unit tests fail. But maybe that's not a bad thing. The build and push docker now requires both build-and-test as well as regression-test to succeed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation or Problem
Fake PR to test CI regression fix: this branch should fail regression tests because it automatically stops RMG runs after 3 iterations