Skip to content

Conversation

@kyriv1980
Copy link
Contributor

@kyriv1980 kyriv1980 commented Oct 24, 2025

This Pr resolves issues #31044, #30037, #31799 which:

  • Fix a bug with the crossflow aux variable population inside the implicit solver and regold as needed.

  • Clean-up and finalize the implicit solver (remove the full-monolithic method) since we opt to lag the enthalpy solution.

  • Some cosmetic/readability improvements.

This PR is an improved version of : #31500

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@kyriv1980 kyriv1980 force-pushed the delete_full_monlithic branch from 2c9bea4 to 7b09a19 Compare October 24, 2025 21:45
@moosebuild
Copy link
Contributor

moosebuild commented Oct 25, 2025

Job Documentation, step Docs: sync website on ce6ccf5 wanted to post the following:

View the site here

This comment will be updated on new commits.

@moosebuild
Copy link
Contributor

moosebuild commented Oct 25, 2025

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

Framework coverage

ac8805 #31785 ce6ccf
Total Total +/- New
Rate 85.97% 85.97% -0.00% -
Hits 124979 124978 -1 0
Misses 20389 20390 +1 0

Diff coverage report

Full coverage report

Modules coverage

Subchannel

ac8805 #31785 ce6ccf
Total Total +/- New
Rate 94.74% 94.04% -0.70% 100.00%
Hits 6030 5881 -149 145
Misses 335 373 +38 0

Diff coverage report

Full coverage report

Full coverage reports

Reports

This comment will be updated on new commits.

@GiudGiud GiudGiud self-assigned this Oct 27, 2025
@kyriv1980 kyriv1980 force-pushed the delete_full_monlithic branch 2 times, most recently from 00c2f09 to 1c7b477 Compare October 27, 2025 17:07
@kyriv1980 kyriv1980 requested a review from GiudGiud October 27, 2025 19:11
@kyriv1980 kyriv1980 force-pushed the delete_full_monlithic branch from 1c7b477 to 227290b Compare October 28, 2025 17:19
@kyriv1980 kyriv1980 force-pushed the delete_full_monlithic branch 4 times, most recently from d1e1fca to a49a2ac Compare October 28, 2025 22:47
@kyriv1980 kyriv1980 force-pushed the delete_full_monlithic branch from a49a2ac to df34e81 Compare October 29, 2025 15:38
@moosebuild
Copy link
Contributor

Job GCC min debug on df34e81 : invalidated by @kyriv1980

@kyriv1980 kyriv1980 requested a review from GiudGiud October 29, 2025 21:13
Copy link
Contributor

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

@tanoret there are changes in the validation suite, do you want to look this over?

@kyriv1980 kyriv1980 force-pushed the delete_full_monlithic branch from df34e81 to f15c50f Compare October 30, 2025 15:46
@moosebuild
Copy link
Contributor

Job Docker moose-dev-openmpi on f15c50f : invalidated by @kyriv1980

@moosebuild
Copy link
Contributor

Job Apptainer moose-openmpi on f15c50f : invalidated by @kyriv1980

@kyriv1980 kyriv1980 force-pushed the delete_full_monlithic branch 2 times, most recently from 6e2eabb to ba4f3cf Compare October 30, 2025 17:34
@kyriv1980 kyriv1980 force-pushed the delete_full_monlithic branch from ba4f3cf to 5e366b0 Compare October 30, 2025 18:58
@kyriv1980 kyriv1980 requested a review from GiudGiud November 6, 2025 16:16
kyriv1980 added a commit to kyriv1980/moose that referenced this pull request Nov 7, 2025
@moosebuild
Copy link
Contributor

Job Python 3.11 on d06e97b : invalidated by @kyriv1980

Copy link
Contributor

@tanoret tanoret left a comment

Choose a reason for hiding this comment

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

All good for the CSV. All changes are within experimental uncertainties of the associated tests.

@moosebuild
Copy link
Contributor

Job Test, step Results summary on ce6ccf5 wanted to post the following:

Framework test summary

Compared against ac88058 in job civet.inl.gov/job/3368843.

Removed tests

Added tests

Run time changes

Modules test summary

Compared against ac88058 in job civet.inl.gov/job/3368843.

Removed tests

Test Time (s)
subchannel/test:problems/SFR/sodium-19pin.test_full_monolithic 1.67
subchannel/test:problems/psbt.psbt_regression_test_full_monolithic_staggered 1.46
subchannel/test:problems/psbt.psbt_regression_test_full_monolithic 1.31

Added tests

Run time changes

Test Base (s) Head (s) +/-
subchannel/validation/ORNL_19_pin.ORNL-19 10.63 3.35 -68.45%

@GiudGiud GiudGiud changed the title Delete full monolithic solver Delete full monolithic solver & re-organize problems using lambdas Nov 8, 2025
@GiudGiud
Copy link
Contributor

GiudGiud commented Nov 8, 2025

great cleanup

@GiudGiud GiudGiud merged commit ee1c546 into idaholab:next Nov 8, 2025
68 of 69 checks passed
@kyriv1980 kyriv1980 deleted the delete_full_monlithic branch November 10, 2025 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants