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

NOXSolver: clear pending_exception #16028

Closed
wants to merge 1 commit into from

Conversation

peterrum
Copy link
Member

@@ -961,6 +961,7 @@ namespace TrilinosWrappers
n_jacobian_applications = 0;
n_nonlinear_iterations = 0;
n_last_linear_iterations = 0;
pending_exception = nullptr;
Copy link
Member Author

Choose a reason for hiding this comment

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

Needed to be able to reuse the solver.

Copy link
Member

Choose a reason for hiding this comment

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

This doesn't quite seem right. If there is still an exception pending, we should assert. Can you create a testcase that shows how this can happen?

@peterrum
Copy link
Member Author

@bangerth I don't think the call_and_possibly_capture_exception() logic is correct in NOXSolver. Let's assume that the linear solver fails and NOX does not stop but decreases the step size!?

@peterrum
Copy link
Member Author

@bangerth Any feedback. The NOXSolver on master is not working in our application after the update.

@bangerth
Copy link
Member

@peterrum Apologies for beging slow to get to this -- I spent the weekend from Friday afternoon to this morning in the mountains.

The logic is that call_and_possibly_capture_exception() saves any exceptions for later use because, at least as far as I understand, NOX does not have the concept of "recoverable exceptions". At least in other libraries, if there are no recoverable exceptions, as soon as a user callback returns an error code, the library in question stops any operations and returns to user code (in our case, to deal.II code) where we would note that an exception has been saved, and re-throw it. That happens here:
https://github.com/dealii/dealii/blob/master/include/deal.II/trilinos/nox.templates.h#L1132-L1216

The logic here implicitly assumes that if NOX returns success (NOX::StatusTest::Converged), then there should not be a pending exception. That could also usefully be asserted, in line 1146 just before the return statement.

So, if NOX does have the ability to recover from exceptions, then that would be useful to know -- ideally with a testcase that demonstrates that, and by adding the assertions I mention above.

@bangerth
Copy link
Member

bangerth commented Oct 5, 2023

@peterrum Ping?

@bangerth
Copy link
Member

@peterrum Ping again?

@peterrum
Copy link
Member Author

Ping?

Ping again?

@vovannikov Is working on a simplified test that triggers the problem.

@vovannikov
Copy link
Contributor

The nullification of the pending_exception won't resolve the issue we observed in our code. The problem comes from a bit different part of NOX, see #16329.

@peterrum peterrum closed this Jan 1, 2024
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

3 participants