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

Trying to improve adaptive linear solver tolerance selection #3163

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

paveltomin
Copy link
Contributor

@paveltomin paveltomin commented Jun 7, 2024

  1. Pull out hard-codded params to be able to modify them from input:
- strongestTol = 1e-8 (default value)
- adaptiveGamma = 0.1
- adaptiveExponent = 1.0
  1. Adjust altKrylovTol logic. Not sure that new one is good but old one is really confusing.
  2. Add output to see what tolerance is selected and why (triggered by logLevel>0 in linear solver params).

@paveltomin
Copy link
Contributor Author

paveltomin commented Jun 7, 2024

I guess this PR is more a discussion point for now.
I found that adaptive tolerance approach is both quite useful for large scale realistic cases and at the same time the algorithm is quite confusing.
Trying to adjust it to make tolerance choice more understandable.
Please let me know what you think.

normRatio = 1;

real64 newKrylovTol = gamma*std::pow( normRatio, exponent );
real64 altKrylovTol = gamma*std::pow( oldNewtonNorm, exponent );
Copy link
Contributor Author

@paveltomin paveltomin Jun 7, 2024

Choose a reason for hiding this comment

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

this one is confusing, i replaced oldNewtonNorm but old linear solver tolerance, but not quite sure about that
oldNewtonNorm squared is usually way too tight

@victorapm
Copy link
Contributor

Thanks, Pavel! Just checking... The algorithm is described here, correct?

@paveltomin
Copy link
Contributor Author

Thanks, Pavel! Just checking... The algorithm is described here, correct?

I think yes, but because I don't have access I can't say for sure.
I was reading this one https://softlib.rice.edu/pub/CRPC-TRs/reports/CRPC-TR94463.pdf which should be the same I guess.

@paveltomin
Copy link
Contributor Author

any thoughts?

@victorapm
Copy link
Contributor

I don't have much to add for now but I'll keep thinking about this. @oseikuffuor1 could you chime in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants