Ver-1kc-1

[lkadz]

(Ref. #12)

[simopier]

Hi @Lee01Atom, thanks for pushing these new verification cases!

The conflicts come from the fact that you are probably not basing your PR on the latest version of devel.

Before creating a branch, you need to make sure you are on branch devel and that it is up to date. I'm sharing the instructions below for future reference, and then I am sharing the instructions you should be able to resolve this without re-creating a new branch.

git checkout devel
git fetch upstream && git reset --hard upstream/devel && git submodule update # this deletes any new local changes made in the devel branch. 
git checkout -B new_branch_name

When you do this, it creates a new branch that is up to date with devel, and you can start making changes. This is consistent with the instructions we provide in https://mooseframework.inl.gov/TMAP8/getting_started/contributing.html.

Now, in your situation, since you already have a branch with local changes, you should do:

git checkout ver-1kc
git status # make sure you do not have any non-committed changes. If you do, commit them
git fetch upstream && git rebase upstream/devel && git submodule update

In that process, you will see that it runs into conflicts as it goes through new commits, and it will list the problematic files. The best way to deal with these is to open the file in VSCode and use their resolve in editor feature to what you want it to be.
In this PR, I think the only conflict will be that you are adding a new line in the table of verification cases in the V&V documentation page, but a new line was also added for the ver-1kb case, so you just have to make sure it includes both when you resolve the conflict, and you'll be OK.
The add the files you have updated, commit them, and continue to follow the instructions until you are done with the rebase. At that point, your branch will be up to date with your new commits at the top.
You can check that everything is in order using git log.

Once this is all done, you can add more commits as needed and push to this PR. Note that you might have to use the -f option to force push. It will look something like this:

git push origin ver-1kc -f

instead of the regular

git push origin ver-1kc

Please don't start using the force push option by default, as it can sometimes remove some important history if one is not careful.

Let me know if you have any questions.

[simopier]

A preliminary review shows me that things look OK.
But instead of using K=1/RT and K=10/RT, you should use K=1/sqrt(RT) and K=10/sqrt(RT) to account for n=0.5.
Just like for ver-1ka, , K=1/sqrt(RT) would not lead to a concentration jump, but K=10/sqrt(RT) would, hence showcasing TMAP8's ability to handle both. Make sure to update the documentation, python script (including comments), input files, etc.

[moosebuild]

Job Documentation, step Sync to remote on ddacbaf wanted to post the following:

View the site here

This comment will be updated on new commits.

[simopier]

The documentation failure is due to duplicate requirements in tests. Requirements need to be unique, and I think you forgot to update from Henry to Sievert.

[moosebuild]

Job Coverage, step Generate coverage on ddacbaf wanted to post the following:

Coverage

Coverage did not change

Full coverage report

This comment will be updated on new commits.

[simopier]

Looks good, I just have a few comments, suggestions, and questions.

[simopier]

Things look good, but the test indeed takes too long to run (that's why debug is failing).

I think you should create some heavy tests, as was done in https://mooseframework.inl.gov/TMAP8/verification_and_validation/ver-1d.html.

As you look at how it was done in the other case, here's what to pay attention to:

• Reduce the tolerances of the input file to make it accurate. You can also further refine the mesh if you want. This will be your heavy test, meaning it is expected to take longer than 2 seconds to run.
• You can then edit the test specification file to create a new test that uses the same input file but adapts it using cli_agrs to (1) use a coarser mesh and (2) maybe use looser convergence criteria if needed - all to make the tests run in less than 2 seconds. We only need a csv test for this one.
• In the test specification file, make sure to mark the heavy tests (both csv and exodus) with heavy = true.
• to run the heavy tests locally, do ./run_tests -j4 --heavy. To run this case specifically, do ./run_tests -j4 --heavy --re=ver-1kc
• The python script should still use the output from the heavy test to plot the results.
• Make sure you update the input file section of the documentation.

Let me know if you have any questions.

[simopier]

Good job on adding the heavy tests and the light tests! I left some suggestions for improvements.

[simopier]

Thank you for this contribution!