-
Notifications
You must be signed in to change notification settings - Fork 37
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
Update for 2D GEC Reference Cell Validation #45
Conversation
…o current test for new BCs
…ns, and adding a test folder for conference input files
Currently in the middle of a review, but noticed that the current failures are the result of warnings being treated as errors in the Zapdos testing recipe on CIVET. Those need to be corrected before Zapdos will be allowed to build. See https://civet.inl.gov/job/417790/ and https://civet.inl.gov/job/417791/ for more info on the job output. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work! Most of my comments are clean-up and typo related. A general comment regarding testing -- test blocks can take in the command-line arguments that allow you to make quick changes to the input file using cli_args=
. This allows you to have one base input file, that you then modify to test a specific version (over having multiple copies of the same file minus tiny changes). For example, in your Lymberopoulos_with_argon_metastables_2D_At100mTorr_CourseMesh.i
Exodiff test, you could have created a test for Lymberopoulos_with_argon_metastables_2D_At100mTorr.i
but with the argument cli_args='Mesh/file='GEC_mesh_coarse.msh'
that would have modified that line temporarily for the test and used the coarse mesh. It tends to reduce input file clutter while still doing the testing you want.
…po/clean-up comments
I just push the new commit with Casey's requested changes and fixed the CIVET warning for EconomouDielectricBC.C. I think the other warnings were for CRANE kernels. @keniley1 please let me know if you want me to do a pull request to fix those warning (or if I just have to update Zapdos' version of CRANE, if they have already been fixed). |
I also missed one more CIVET for Zapdos that I will fix. |
Thanks Corey! |
@cticenhour @csdechant |
…ative tolerance of the Lymberopoulos_with_argon_metastables_2D_At100mTorr_CourseMesh test
Mac test failed with error "Automatic scaling requires a PETSc version of 3.9.0 or greater" for the Conference_Syntax_Tests.Lymberopoulos_with_argon_metastables_2D_At1Torr test (which is weird since the course mesh Exodiff version of that test passed. I will just comment out the automatic scaling for that syntax test for now (so Casey can update the website), if no one has any objections. |
Instead in the tests file for that test put:
petsc_version >= ‘3.9.0’
Don’t just comment it out. We want automatic scaling present in whatever test suite supports it.
… On Oct 29, 2019, at 6:46 PM, csdechant ***@***.***> wrote:
Mac test failed with error "Automatic scaling requires a PETSc version of 3.9.0 or greater" for the Conference_Syntax_Tests.Lymberopoulos_with_argon_metastables_2D_At1Torr test (which is weird since the course mesh Exodiff version of that test passed. I will just comment out the automatic scaling for that syntax test for now (so Casey can update the website), if no one has any objections.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Thank you for the information. Would I state that outside of the test block? Like: |
No, you'd put it inside the test that you want the condition to apply to. For example:
|
No within the sub-block of whatever test includes automatic scaling
… On Oct 29, 2019, at 11:37 PM, csdechant ***@***.***> wrote:
Thank you for the information. Would I state that outside of the test block? Like:
petsc_version >= ‘3.9.0’
[Tests]
.
.
.
[]
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
…h_argon_metastables_2D_At1Torr syntax test
This update will add the following: