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.

Sign up for GitHub

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

Jump to bottom

Add verification case val-2d #204

Merged
merged 14 commits into from
Dec 4, 2024
Merged

Add verification case val-2d #204

merged 14 commits into from
Dec 4, 2024

Conversation

lin-yang-ly
Copy link
Collaborator

@lin-yang-ly lin-yang-ly commented Oct 30, 2024 •
edited
Loading

(Ref. #12)
(Close. #203)

@lin-yang-ly
Copy link
Collaborator Author

@simopier I just completed the validation case val-2d. Just let me know if you have any suggestions!

BTW, the running time of the validation case is still around 9s after I optimized the mesh and time steps. I will continue to find a way to lower the running time, and please let me know if you have any suggestions for that.

@moosebuild
Copy link

moosebuild commented Oct 30, 2024 •
edited
Loading

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

View the site here

This comment will be updated on new commits.

@moosebuild
Copy link

moosebuild commented Oct 30, 2024 •
edited
Loading

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

Coverage

Coverage did not change

Full coverage report

This comment will be updated on new commits.

@simopier simopier self-assigned this Oct 30, 2024
@simopier simopier added the V&V Relevant to V&V label Oct 30, 2024
@lin-yang-ly lin-yang-ly force-pushed the add_val2d branch 3 times, most recently from add91f2 to 2fd23dd Compare November 5, 2024 17:20
@lin-yang-ly
Copy link
Collaborator Author

@simopier I found a issue on the RMSPE calculation in val-2d, so I fix it in the latest commits. However, the PR met an "TIMEOUT" error from "ver-1c" during the "Test Debug" check. Do I need to do something for "ver-1c" or leave it alone?

@simopier
Copy link
Collaborator

simopier commented Nov 5, 2024

Definitely leave it alone here.

simopier
simopier requested changes Nov 11, 2024
View reviewed changes
Copy link
Collaborator

@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.

Thank you for this contribution, @lin-yang-ly! Let me know if you have any questions.

test/tests/val-2d/comparison_val-2d.py Outdated Show resolved Hide resolved
test/tests/val-2d/comparison_val-2d.py Outdated Show resolved Hide resolved
test/tests/val-2d/comparison_val-2d.py Outdated Show resolved Hide resolved
test/tests/val-2d/val-2d.i Outdated Show resolved Hide resolved
test/tests/val-2d/val-2d.i Outdated Show resolved Hide resolved
doc/content/verification_and_validation/val-2d.md Outdated Show resolved Hide resolved
doc/content/verification_and_validation/val-2d.md Outdated Show resolved Hide resolved
test/tests/val-2d/tests Outdated Show resolved Hide resolved
test/tests/val-2d/tests Outdated Show resolved Hide resolved
test/tests/val-2d/tests Outdated Show resolved Hide resolved
@lin-yang-ly lin-yang-ly force-pushed the add_val2d branch 4 times, most recently from 64194a8 to c87a3a1 Compare November 18, 2024 20:13
@lin-yang-ly
Copy link
Collaborator Author

@simopier I updated the heavy tests with coarser mesh to avoid the "heavy test issue" and the RMSPE don't have much difference. Right now, We can use time_step_interval of 20 instead of a strange large number. Also, I clean the commits history and it is ready for review!

simopier
simopier requested changes Nov 24, 2024
View reviewed changes
Copy link
Collaborator

@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.

Some more comments, the main one being that the current TMAP8 results do not look that great, maybe the time step is too large.

doc/content/verification_and_validation/val-2d.md Outdated Show resolved Hide resolved
doc/content/verification_and_validation/val-2d.md Outdated Show resolved Hide resolved
doc/content/verification_and_validation/val-2d.md Outdated Show resolved Hide resolved
doc/content/verification_and_validation/val-2d.md Outdated Show resolved Hide resolved
test/tests/val-2d/tests Outdated Show resolved Hide resolved
test/tests/val-2d/tests Outdated Show resolved Hide resolved
doc/content/verification_and_validation/val-2d.md Show resolved Hide resolved
test/tests/val-2d/tests Outdated Show resolved Hide resolved
test/tests/val-2d/tests Outdated Show resolved Hide resolved
lin-yang-ly and others added 12 commits December 2, 2024 09:19
@lin-yang-ly
Add val-2d:
ec173f2 
- add input and gold files for model
- add documentation
- add python script

(Ref. idaholab#203)
@lin-yang-ly
update documentation
60dea9e 
update csv file

update tests
@lin-yang-ly
update limited model with coarse mesh and time step and update docume…
e30400d 
…ntation
@lin-yang-ly
update tolerance for test recover check
cc1df6f
@lin-yang-ly
Add "recover = false" in test case
8d9b51a 
Ref. idaholab#196
@lin-yang-ly
fix the issue during RMSPE calculation
336f27c
@lin-yang-ly
update RMSPE in documentation
0af5cf4
@lin-yang-ly @simopier
Apply suggestions from code review
fb5ec0d 
Co-authored-by: Pierre-Clement Simon <[email protected]>
@lin-yang-ly
Update case with suggestions
48e28d7
@lin-yang-ly
solve auto-checks issue and update RMSPE
d6d7d31
@lin-yang-ly @simopier
Apply suggestions from code review
6e287f5 
Co-authored-by: Pierre-Clement Simon <[email protected]>
@lin-yang-ly
make time interval finer
0d37827
simopier
simopier requested changes Dec 2, 2024
View reviewed changes
Copy link
Collaborator

@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.

An additional comment, and some older comments that were not fully addressed.

test/tests/val-2d/val-2d.i Show resolved Hide resolved
test/tests/val-2d/val-2d.i Outdated Show resolved Hide resolved
doc/content/verification_and_validation/val-2d.md Outdated Show resolved Hide resolved
@lin-yang-ly @simopier
Apply suggestions from code review
48b5897 
Co-authored-by: Pierre-Clement Simon <[email protected]>
@lin-yang-ly
Copy link
Collaborator Author

@simopier I have lowered the tolerance to 8e-9 from 1e-6. Further reducing the tolerance could increase the coarse simulation running time to over 3 seconds.

simopier
simopier approved these changes Dec 4, 2024
View reviewed changes
Copy link
Collaborator

@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.

Looks good to me, thank you very much for your contribution, @lin-yang-ly!

@simopier simopier marked this pull request as ready for review December 4, 2024 02:16
@simopier simopier merged commit 5f7ffd6 into idaholab:devel Dec 4, 2024
9 checks passed
@lin-yang-ly lin-yang-ly deleted the add_val2d branch December 4, 2024 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simopier simopier simopier approved these changes

Assignees

@simopier simopier

Labels
V&V Relevant to V&V
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lin-yang-ly @moosebuild @simopier