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

Fixed a bug in CFiniteElementStd::Assemble_RHS_M() #93

Merged
merged 4 commits into from
Apr 5, 2018

Conversation

wenqing
Copy link
Member

@wenqing wenqing commented Apr 4, 2018

and some other minor changes.

@wenqing
Copy link
Member Author

wenqing commented Apr 4, 2018

@norihiro-w I am preparing the final version of ogs5, v5.8, which will include some material models developed by BGR and some bug fixing. If there is any fundamental changes, I would like to have your comment. For minor bug fixing and changes, I may merge it quickly.

@@ -795,13 +795,16 @@ double CRFProcessDeformation::Execute(int loop_process_number)
{
pcs_absolute_error[0] = NormU;
pcs_relative_error[0] = pcs_absolute_error[0] / Tolerance_global_Newton;
cpl_max_relative_error = pcs_relative_error[0];
// To be improved by overloading CRFProcess::CalcIterationNODError
cpl_max_relative_error = pcs_absolute_error[0]; //pcs_relative_error[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

two comments

  • why cannot you use pcs_relative_error[0]?
  • it's quite confusing if cpl_max_relative_error is actually absolute error. isn't there cpl_max_abs_error?

Copy link
Member Author

Choose a reason for hiding this comment

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

cpl_max_relative_error is only set here under if (ite_steps == 1). If cpl_max_relative_error is set to pcs_relative_error[0]( = (pcs_absolute_error[0] / Tolerance_global_Newton), the coupling convergence criterion of cpl_max_relative_error < 1 is never satisfied for some cases. Anyway, this implementation is not correct. Therefore, I made this change just to make sure that computation of the modelling of M related coupled processes can keep running, while its coupling convergence is controlled by other processes. As you can see in the comment above the changed line, the method in CRFProcess::CalcIterationNODError will be used to calculate the coupling error of M process.

Copy link
Member Author

Choose a reason for hiding this comment

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

@norihiro-w Thanks for your comments. I am going to merge this PR.

@norihiro-w
Copy link
Contributor

@wenqing Thanks for preparing the new version! I have only one minor comment. If that is resolved, please go ahead.

@wenqing wenqing merged commit 29f824b into ufz:master Apr 5, 2018
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.

2 participants