Add verification case ver-1m#411
Conversation
simopier
left a comment
There was a problem hiding this comment.
Thank you! I see that you have some conflicts with the bib file.
You will want to run:
git fetch upstream && git rebase upstream/devel && git submodule update
and resolve the conflicts locally. What probably happened is that you added the references at the bottom of the files, and other people did too since you started your changes, and so now github is not sure what to do because the changes are not consistent.
One easy way to resolve that in the future is to put your references someone in the middle of the files - it's less likely that someone will add them there too.
For now, use the command above to update your local branch and it will tell you what files are conflicting, then you can open them in vscode and resolve the conflicts (make sure not to remove the references others added), then git add the updated files, and follow the directions, which will tell you to do git rebase --continue or something like that. It will open the commit file and you can just save it (:wq) and move on until your version is up to date and ready to be pushed agin.
simopier
left a comment
There was a problem hiding this comment.
Thank you for your contribution @f-griffin!
This looks incredibly good for a first PR, well done!
I have some comments for you, you can find them here and add them to batch and commit them online.
Let me know if you run into any issues doing this.
The precheck tests are currently failing because you did not reference the issue number in your commits (see https://tmap8.inl.gov/getting_started/contributing.html for more information and guidance)
You'll need to add (Ref. #383) in one of your commit messages.
Let me know if you have any questions.
|
|
||
| The material properties and case parameters are provided in [ver-1m_set_up_values]. | ||
|
|
||
| !table id=ver-1m_set_up_values caption=Values of material properties and case geometry for the UZrH hydrogen migration verification problem. |
There was a problem hiding this comment.
Given your note below the table, I recommend adding a Ref. columns to list the relevant reference [!cite](huang01102000) or state that it is an estimate.
There was a problem hiding this comment.
Same comment as in the input file here about temp vs temperature.
You will need to rename the output files too.
There was a problem hiding this comment.
You need to make sure you have tests for all the csv files of interest. Here, you use the profiles mostly, so I recommend testing the profiles csv instead.
And make sure to remove all the csv files that are not tested from the gold folder.
You should be able to do
csvdiff = `filename_1.csv filename_2.csv`
|
Hi @f-griffin, do you have any questions about any of the comments I have left? |
Co-authored-by: Pierre-Clement Simon <pierreclement.simon@gmail.com>
|
Job Precheck, step Python: black format on ae84fe8 wanted to post the following: Python black formattingYour code requires style changes. A patch was generated and copied here. You can directly apply the patch by running the following at the top level of your repository: Alternatively, you can run the following at the top level of your repository: |
|
Job Documentation, step Sync to remote on d920571 wanted to post the following: View the site here This comment will be updated on new commits. |
|
Job Build test summary, step Build test summary on d920571 wanted to post the following: Test summaryCompared against a35a940 in job civet.inl.gov/job/3912142. Added tests
|
Add verification case ver-1m for steady state soret diffusion and conduction
(Ref. #383)