Bugfix: CVODE(S) Inequality Constraints #787
Conversation
|
@drreynolds I've updated the correction modification to address the roundoff issues and resolved the open comments. However, this PR also includes updates from #788 (makes adding logging to the constraint checks easier), so hold off on taking another pass over this PR until that one is merged. |
Conflicts: doc/superbuild/source/developers/source_code/Style.rst test/answers test/unit_tests/logging/test_logging_cvode_lvl5_0.out test/unit_tests/logging/test_logging_cvode_lvl5_1_0.out test/unit_tests/logging/test_logging_cvode_lvl5_1_1.out
Conflicts: test/answers
| CVODES options set via :c:func:`CVodeSetOptions` will overwrite | ||
| any previously-set values. Options are set in the order they are given in | ||
| ``argv`` and, if an option with the same prefix appears multiple times in | ||
| ``argv``, the value of the last occurrence will used. |
There was a problem hiding this comment.
| ``argv``, the value of the last occurrence will used. | |
| ``argv``, the value of the last occurrence will be used. |
| | **CVODE main solver** | | | ||
| +-------------------------------------------------+--------------------------------------------+ | ||
| | Size of CVODE real and integer workspaces | :c:func:`CVodeGetWorkSpace` | |
There was a problem hiding this comment.
Was it intentional that these were changed from CVODES to CVODE here in the CVODES documentation? If not, here's a suggestion to revert:
| | **CVODE main solver** | | | |
| +-------------------------------------------------+--------------------------------------------+ | |
| | Size of CVODE real and integer workspaces | :c:func:`CVodeGetWorkSpace` | | |
| | **CVODES main solver** | | | |
| +-------------------------------------------------+--------------------------------------------+ | |
| | Size of CVODES real and integer workspaces | :c:func:`CVodeGetWorkSpace` | |
| | All CVODE integrator statistics | :c:func:`CVodeGetIntegratorStats` | | ||
| +-------------------------------------------------+--------------------------------------------+ | ||
| | CVODE nonlinear solver statistics | :c:func:`CVodeGetNonlinSolvStats` | |
There was a problem hiding this comment.
As above, if the change from CVODES to CVODE here in the CVODES documentation was unintentional, here's a suggestion to revert:
| | All CVODE integrator statistics | :c:func:`CVodeGetIntegratorStats` | | |
| +-------------------------------------------------+--------------------------------------------+ | |
| | CVODE nonlinear solver statistics | :c:func:`CVodeGetNonlinSolvStats` | | |
| | All CVODES integrator statistics | :c:func:`CVodeGetIntegratorStats` | | |
| +-------------------------------------------------+--------------------------------------------+ | |
| | CVODES nonlinear solver statistics | :c:func:`CVodeGetNonlinSolvStats` | |
| +--------------------------------------------------------------------+------------------------------------------+ | ||
| | **Optional output** | **Function name** | | ||
| +====================================================================+==========================================+ | ||
| | Size of IDA real and integer workspace | :c:func:`IDAGetWorkSpace` | |
There was a problem hiding this comment.
Was this IDAS -> IDA change intentional (here in the IDAS docs)? If not, here's a suggestion to revert:
| | Size of IDA real and integer workspace | :c:func:`IDAGetWorkSpace` | | |
| | Size of IDAS real and integer workspace | :c:func:`IDAGetWorkSpace` | |
| sunfprintf_long(outfile, fmt, SUNFALSE, "Constraint fails", | ||
| cv_mem->constraint_fails); | ||
| sunfprintf_long(outfile, fmt, SUNFALSE, "Constraint corrections", | ||
| cv_mem->constraint_corrections); |
There was a problem hiding this comment.
Do you really want to print these by default, or should this be inside a conditional based on whether constraint-handling is used, e.g.
| sunfprintf_long(outfile, fmt, SUNFALSE, "Constraint fails", | |
| cv_mem->constraint_fails); | |
| sunfprintf_long(outfile, fmt, SUNFALSE, "Constraint corrections", | |
| cv_mem->constraint_corrections); | |
| if (cv_mem->cv_constraints == NULL) | |
| { | |
| sunfprintf_long(outfile, fmt, SUNFALSE, "Constraint fails", | |
| cv_mem->constraint_fails); | |
| sunfprintf_long(outfile, fmt, SUNFALSE, "Constraint corrections", | |
| cv_mem->constraint_corrections); | |
| } |
(and similar question for CVODES, IDA, and IDAS)
| sunfprintf_long(outfile, fmt, SUNFALSE, "Constraint fails", | ||
| cv_mem->constraint_fails); | ||
| sunfprintf_long(outfile, fmt, SUNFALSE, "Constraint corrections", | ||
| cv_mem->constraint_corrections); |
There was a problem hiding this comment.
As with CVODE, do you really want to print these by default?
| sunfprintf_long(outfile, fmt, SUNFALSE, "Constraint fails", | ||
| IDA_mem->constraint_fails); | ||
| sunfprintf_long(outfile, fmt, SUNFALSE, "Constraint corrections", | ||
| IDA_mem->constraint_corrections); |
There was a problem hiding this comment.
As with CVODE, do you really want to print these by default?
| sunfprintf_long(outfile, fmt, SUNFALSE, "Constraint fails", | ||
| IDA_mem->constraint_fails); | ||
| sunfprintf_long(outfile, fmt, SUNFALSE, "Constraint corrections", | ||
| IDA_mem->constraint_corrections); |
There was a problem hiding this comment.
As with CVODE, do you really want to print these by default?
| if ((SUNRabs(cv_mem->cv_h) <= cv_mem->cv_hmin * ONEPSM) || | ||
| (*step_constraint_fails == cv_mem->max_constraint_fails)) | ||
| { | ||
| SUNLogInfo(CV_LOGGER, "end-constraint-check", "status = failed max attempts"); |
There was a problem hiding this comment.
Minor item: this error message could also be displayed if |h|==hmin, so couldn't this "status" be misleading? The same comment is relevant for CVODES, IDA, and IDAS.
Fix a bug in the CVODE(S) inequality constraint handling where the predicted state was used to compute the step size reduction factor instead of the prior solution which could lead to an incorrect reduction in the step size or, when the prediction violates the constraints, an infinitely large step size in the next step attempt. (Fixes #702)
Add a unit test for inequality constraint handling with all the integrators.
In CVODE(S) and IDA(S), separate inequality constraint check from the nonlinear solve check.
Add functions to set the maximum number of inequality constraint failures and get total number of failures (constraint failures are no longer included in the number of step failures due to a solver failure).
Add a function to get the number of steps modified but not failed due to constraint violations.
Synchronize some differences between CVODE/CVODES and IDA/IDAS source and docs.
Add missing return flag names to
CVodeGetReturnFlagNamein CVODES andIDAGetReturnFlagNamein IDA.