-
Notifications
You must be signed in to change notification settings - Fork 238
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
Update dynamic_core.cc #6168
base: main
Are you sure you want to change the base?
Update dynamic_core.cc #6168
Conversation
The previous code would halt if no solution was found for the inner core, preventing the simulation from proceeding. This limitation made it impossible to observe the Earth's mantle evolution during cooling when the inner core had not yet formed. The updated code addresses this issue. Now, if the inner core has not formed, the simulation continues, allowing the core-mantle boundary (CMB) to cool. If the inner core eventually forms during the simulation, its radius will be recorded in the statistics.
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.
Hi @Francyrad, I am glad you found a solution for your forum post and thanks a lot for providing a fix! I have a few comments on your changes, because I think you found the problem (an overzealous error check that didnt allow for a fully molten or fully solid core), but your solution is to disable all error checks, while I would prefer to only adjust the crucial check to allow fully solid or fully molten cores.
Let me know in case some of my comments are not clear or you want to discuss something.
else if (dT0*dT1<0.) | ||
{ | ||
|
||
} else if (dT2 <= 0. && dT0 <= 0.) { |
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.
Please follow our usual coding style of putting curly braces on separate lines, otherwise our style testers will complain.
<< " R=[" << R_0 / 1e3 << "," << R_2 / 1e3 << "] (km)" | ||
<< " dT0=" << dT0 << ", dT2=" << dT2 << std::endl | ||
<< "Q_CMB=" << core_data.Q << std::endl | ||
<< "The inner core is not formed yet or the core is solidus." << std::endl; |
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.
What do you mean by core is solidus
? Do you mean the temperature at the inner-outer core boundary has reached exactly the solidus? Please extend the warning message to make this clearer.
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.
Also I do not think that the code can end up here if the inner core has not formed yet. If the inner core has not formed yet both dT0 and dT2 should be either 0 or positive and we should end up in line 437, but not here. I think something else must be causing the error message you reference in your forum post. From what I see in your post it is not this line that throws the exception, but the assert in line 494/495. See my comment for how to fix that line below. Once you have changed that, could you test if this line here needs any changes at all or if we can just leave the old version?
R = R_0; | ||
T = get_Tc(R); | ||
X = get_X(R); | ||
return true; // Continua la simulazione |
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.
Please keep all comments in English ;-)
T = get_Tc(R); | ||
X = get_X(R); |
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.
Remove these lines, R
has been set correctly to R_1
in line 475 (note that for the case of a fully molten core R_1=R_0
in line 437), T
and X
have already been set correctly in lines 476 and 477.
<< " R=[" << R_0/1e3 << "," << R_2/1e3 << "]" << "(km)" | ||
<< " dT0=" << dT0 << ", dT2=" << dT2 << std::endl | ||
<< "Q_CMB=" << core_data.Q << std::endl; | ||
AssertThrow(false, ExcMessage("[Dynamic core] No inner core radius solution found!")); |
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.
I think this is the crucial line and you are correct this AssertThrow
was wrong before. However, I think your solution is not optimal either, because it looks as if something unexpected happened. However the if-branches above (line 479 and line 484) simply did not consider the case of a fully molten or fully solid core. If you compare lines 479/484 with the conditions in lines 434 and 440, the case of dT0<=0 && dT2<=0
and dT0=>0 && dT2=>0
are clearly expected to work correctly, it is just not considered an acceptable solution here. Your solution is to disable all the error-checks, but I would propose to instead explicitly allow the cases of fully solid and fully molten core. I.e. I would propose the following structure:
if (dT0<0. && dT2>0.)
{
// Core partially molten, freezing from the inside, normal solution
return true;
}
else if (dT0>0. && dT2<0.)
{
// Core partially molten, snowing core solution
return false;
}
else if (dT0 >= 0. && dT2 >=0.)
{
// Core fully molten, normal solution.
return true;
}
else if (dT0 <= 0. && dT2 <=0.)
{
// Core fully solid, normal solution.
return true;
}
The previous code would halt if no solution was found for the inner core, preventing the simulation from proceeding. This limitation made it impossible to observe the Earth's mantle evolution during cooling when the inner core had not yet formed.
The updated code addresses this issue. Now, if the inner core has not formed, the simulation continues, allowing the core-mantle boundary (CMB) to cool. If the inner core eventually forms during the simulation, its radius will be recorded in the statistics.
Pull Request Checklist. Please read and check each box with an X. Delete any part not applicable. Ask on the forum if you need help with any step.
Describe what you did in this PR and why you did it.
Before your first pull request:
For all pull requests:
For new features/models or changes of existing features: