Skip to content
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

Add Ver 1jb to TMAP8's V&V suite #185

Merged
merged 16 commits into from
Oct 24, 2024
Merged

Add Ver 1jb to TMAP8's V&V suite #185

merged 16 commits into from
Oct 24, 2024

Conversation

simopier
Copy link
Collaborator

Ref. #12
Ref. #145

@moosebuild
Copy link

moosebuild commented Oct 13, 2024

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

View the site here

This comment will be updated on new commits.

@simopier
Copy link
Collaborator Author

simopier commented Oct 13, 2024

EDIT:

this issue was solved. It came from the fact that both tests added in this case were writing checkpoint files with the same name. Hence, depending on the timeline in which part 1 and recover happened, one test could end up using the checkpoint results from the other test.

This PR is fully ready for review.


Somehow, the recover test for the basic ver-1jb csv diff case fails.

in the normal test, I locally get:

Time Step 54, time = 1.60944e+09, dt = 3.15576e+07
 0 Nonlinear |R| = 2.411274e-09
      0 Linear |R| = 2.411274e-09
      1 Linear |R| = 7.021067e-25
 1 Nonlinear |R| = 4.598546e-24
  Nonlinear solve converged due to CONVERGED_FNORM_RELATIVE iterations 1
 Solve Converged!

Outlier Variable Residual Norms:
  tritium_trapped_concentration_scaled: 4.275653e-24

but when I run in recover mode, I get:

Time Step 54, time = 1.60944e+09, dt = 3.15576e+07
 0 Nonlinear |R| = 2.412682e-09
      0 Linear |R| = 2.412682e-09
      1 Linear |R| = 3.107300e-24
 1 Nonlinear |R| = 1.280678e-12
      0 Linear |R| = 1.280678e-12
      1 Linear |R| = 2.833365e-28
 2 Nonlinear |R| = 4.801294e-14
      0 Linear |R| = 4.801294e-14
      1 Linear |R| = 2.043024e-29
 3 Nonlinear |R| = 1.800018e-15
      0 Linear |R| = 1.800018e-15
      1 Linear |R| = 3.814858e-31
 4 Nonlinear |R| = 6.748313e-17
      0 Linear |R| = 6.748313e-17
      1 Linear |R| = 1.430223e-32
 5 Nonlinear |R| = 2.529960e-18
      0 Linear |R| = 2.529960e-18
      1 Linear |R| = 5.706847e-34
 6 Nonlinear |R| = 9.484888e-20
  Nonlinear solve converged due to CONVERGED_FNORM_RELATIVE iterations 6
 Solve Converged!

Outlier Variable Residual Norms:
  tritium_mobile_concentration_scaled: 9.484888e-20

and the time step ends up being different.

To be even more confusing, the case with equivalent concentrations of mobile and trapped tritium runs well in recover mode, when the only difference is the initial concentration of mobile tritium and trap_per_free value.

Any ideas or suggestions @cticenhour, @chaibhave, @lin-yang-ly?

Despite this issue, this PR is ready for review.

@moosebuild
Copy link

moosebuild commented Oct 15, 2024

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

Coverage

Coverage did not change

Full coverage report

This comment will be updated on new commits.

Copy link
Collaborator

@chaibhave chaibhave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically good to go except for some documentation and python code changes.

test/tests/ver-1jb/ver-1jb.i Outdated Show resolved Hide resolved
test/tests/ver-1jb/ver-1jb.i Outdated Show resolved Hide resolved
test/tests/ver-1jb/ver-1jb.i Outdated Show resolved Hide resolved
test/tests/ver-1jb/ver-1jb.i Show resolved Hide resolved
test/tests/ver-1jb/ver-1jb.i Outdated Show resolved Hide resolved
doc/content/verification_and_validation/ver-1jb.md Outdated Show resolved Hide resolved
test/tests/ver-1jb/comparison_ver-1jb.py Outdated Show resolved Hide resolved
test/tests/ver-1jb/comparison_ver-1jb.py Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@simopier simopier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a review, just responses to @chaibhave's comments.

test/tests/ver-1jb/ver-1jb.i Show resolved Hide resolved
test/tests/ver-1jb/ver-1jb.i Outdated Show resolved Hide resolved
Copy link
Collaborator

@chaibhave chaibhave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cticenhour ready for your review, I'm done with all reviews. The comments left unresolved you can resolve, I'm not on the CCB yet so I don't have permissions for it.

Copy link
Member

@cticenhour cticenhour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a few items I'd like addressed, and I resolved the remaining comments from @chaibhave that were marked as completed.

doc/content/verification_and_validation/ver-1jb.md Outdated Show resolved Hide resolved
doc/content/verification_and_validation/ver-1jb.md Outdated Show resolved Hide resolved
doc/content/verification_and_validation/ver-1jb.md Outdated Show resolved Hide resolved
doc/content/verification_and_validation/ver-1jb.md Outdated Show resolved Hide resolved
doc/content/verification_and_validation/ver-1jb.md Outdated Show resolved Hide resolved
doc/content/verification_and_validation/ver-1jb.md Outdated Show resolved Hide resolved
doc/content/verification_and_validation/ver-1jb.md Outdated Show resolved Hide resolved
doc/content/verification_and_validation/ver-1jb.md Outdated Show resolved Hide resolved
doc/content/verification_and_validation/ver-1jb.md Outdated Show resolved Hide resolved
@cticenhour cticenhour merged commit ddd32a5 into devel Oct 24, 2024
8 checks passed
@cticenhour cticenhour deleted the ver-1jb branch October 24, 2024 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
V&V Relevant to V&V
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants