Use SNESSetLagJacobianPersists to force recalculating Jacobian#3404
Use SNESSetLagJacobianPersists to force recalculating Jacobian#3404cmacmackin wants to merge 1 commit into
Conversation
Previously when something happened that would require the Jacobian to be recalculated (instead of lagged), SNESSetLagJacobian() would be used to set the lag to 1 for the next time-step. However, this would turn off lagging altogether for that time-step. What we actually want to do is use SNESSetLagJacobianPersists() so that the Jacobian is recalculated at the start of the time-step and then is lagged as usual. This commit changes to doing that. Note that, in practice, the lag wasn't actually being forced to 1 in most cases, because previously there was a line that would set it back to the value from the configurations. As far as I can tell this line shouldn't have been there.
| continue; // Try again | ||
| } | ||
|
|
||
| if (saved_jacobian_lag != 0) { |
There was a problem hiding this comment.
This should restore the previous Jacobian lag on successful SNES convergence. I think this should restore the JacobianPersists setting to its input option. It looks like rescale() will set JacobianPersists to False, but where would it be restored to True (if that's what was in the settings)?
There was a problem hiding this comment.
If rescale determines that JacobianPersists should be false then that applies for the current time-step and it will be reset on the next one.
There was a problem hiding this comment.
Where does the reset happen? This point (around line 1165) is where the Jacobian lag was being reset: Execution reaches this point only after a successful SNES convergence. If this is removed then I don't see where JacobianPersists is reset.
2 participants
Previously when something happened that would require the Jacobian to be recalculated (instead of lagged), SNESSetLagJacobian() would be used to set the lag to 1 for the next time-step. However, this would turn off lagging altogether for that time-step. What we actually want to do is use SNESSetLagJacobianPersists() so that the Jacobian is recalculated at the start of the time-step and then is lagged as usual. This commit changes to doing that.
Note that, in practice, the lag wasn't actually being forced to 1 in most cases, because previously there was a line that would set it back to the value from the configurations. As far as I can tell this line shouldn't have been there.