-
Notifications
You must be signed in to change notification settings - Fork 28
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
Fullapp reentering fix #372
Conversation
FYI, this is the screen output before:
You can notice that there is nothing in the second multiapp run.
|
@@ -136,7 +136,6 @@ | |||
[UserObjects] | |||
[./neutronics_fission_generator] | |||
type = PKAFissionFragmentNeutronics | |||
relative_density = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change here?
There was a problem hiding this 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 valid parameter of PKAFissionFragmentNeutronics. We have a warning during run-time saying a unused param is detected.
I think this is ready for merging. Be warned that @permcody 's PR idaholab/moose#12771 breaks Magpie. |
|
8dc6f4b
to
8bdc2e5
Compare
This closes #377. |
Job Precheck on 8bdc2e5 wanted to post the following: Your code requires style changes. A patch was auto generated and copied here
Alternatively, with your repository up to date and in the top level of your repository:
|
8bdc2e5
to
218236f
Compare
218236f
to
69362a8
Compare
@dschwen I did not see a moose submodule, so the question is which moose is the test against? |
There now is a MOOSE submodule. |
@YaqiWang this is over a year old. I this still needed? |
I assume if MOOSE is up to date, that test will fail. Is that the case? Geese, I thought this had been merged long ago... |
1 similar comment
App test Mammoth is failing with an unrelated failure. I don't think I understand your last comment @YaqiWang, are apptests supposed to fail without this PR? |
This patch is needed for a change in MOOSE. So if your MOOSE is up to date, the regolded test in this PR will fail without this PR. You can verify this. This PR is supposed to fix that failure. I am not expecting an app test failure. I do not have the access to the test results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
F it, I'm meeting this!
A fix for a MOOSE PR idaholab/moose#12773. That PR allows a wrapped transient run with
FullSolveMultiApp
to reenter properly. I saw with that PR, MyTRIM gets executed twice instead of once before. I think this is the right behavior but need you to confirm. Tag @snschune and @dschwen .I did not see MOOSE submodule in magpie, so cannot do a submodule update along this PR. For the same reason, I marked this PR as WIP. I will remove WIP after the PR in MOOSE gets merged. Thanks.