Add validation case 2e in TMAP8 #236
Conversation
- Add tests - Add input files - Add gold files (Ref.idaholab#12)
|
Job Documentation, step Sync to remote on cefd960 wanted to post the following: View the site here This comment will be updated on new commits. |
doc/content/verification_and_validation/figures/val-2e_equipment_schematic.png
Outdated
Show resolved
Hide resolved
Co-authored-by: Pierre-Clement Simon <[email protected]>
Add the suggestions missing to click Co-authored-by: Pierre-Clement Simon <[email protected]>
|
@simopier The PR is ready for review again! I think the reason of two failed tests (Documentation and Coverage) is because of the Sawtooth outage. |
|
@simopier It is ready for review! Just let me know if you have any questions or suggestions. (The failed "Test Debug" is due to the timeout of ver-1c.) |
There was a problem hiding this comment.
Another round of review, which focused on the documentation and the python script.
I made the suggestion to consolidate the input files to minimize redundancy.
Note that I still need to check the model parameters that you are using, and making sure that they are appropriate.
Co-authored-by: Pierre-Clement Simon <[email protected]>
|
@simopier Ready for review and just let me know if you have any questions for the input files. |
Co-authored-by: Pierre-Clement Simon <[email protected]>
|
Ready for review again (if all checks pass) ! |
There was a problem hiding this comment.
I see that you replaced some of the parameters defined at the top of the file by their numerical values since depending on the order in the input file, some things are not properly defined.
We definitely want to avoid that since it makes using cli_args, performing sensitivity analysis, and making sure everything is consistent much more challenging.
Instead of doing this, you should ensure that things are defined only once and that defined parameters are consistently used (rather than replaced by their numerical values).
To do that, you can use the same base/case specify system for .params files as you did here for .i files. .params files only contain the parameters currently listed at the top of the files, without any blocks.
Let me know if you have any question.
|
Thank you for the suggestion! Now I have placed the parameters (Kb, R, temperature) in separate files (parameters_deuterium_gas.params and parameters_three_gases.params) to avoid any order issues. The other parameters remain in the original files to ensure users can easily understand the data. |
Co-authored-by: Pierre-Clement Simon <[email protected]>
|
Typos are updated and ready for next step! |
Refs #12