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

Throw helpful error when unknowns in the initialization system don't have guesses #2874

Closed
wants to merge 11 commits into from

Conversation

hersle
Copy link
Contributor

@hersle hersle commented Jul 18, 2024

Attempt to fix #2873.

First draft is very much a WIP. Want to see which tests fail.

Checklist

  • Appropriate tests were added
  • Any code changes were done in a way that does not break public API
  • All documentation related to code changes were updated
  • The new code follows the
    contributor guidelines, in particular the SciML Style Guide and
    COLPRAC.
  • Any new documentation only uses public API

@hersle hersle force-pushed the error_missing_guesses branch from 1a30e25 to 53e2aad Compare July 18, 2024 10:39
@hersle hersle changed the title Throw helpful error when unknowns Throw helpful error unknowns in the initialization system don't have guesses Jul 18, 2024
@hersle hersle changed the title Throw helpful error unknowns in the initialization system don't have guesses Throw helpful error when unknowns in the initialization system don't have guesses Jul 18, 2024
@ChrisRackauckas
Copy link
Member

Thanks, I did notice in a few cases we were missing an error message somewhere but didn't track down the best spot to put it. This looks correct.

@hersle
Copy link
Contributor Author

hersle commented Jul 18, 2024

Yeah I'm also not 100% sure where to put it. I guess this applies more generally to any NonlinearSystem, so maybe it should go there?

@ChrisRackauckas
Copy link
Member

But an error at that level might be confusing because people might not understand the lack of contextualization, i.e. it would say "u0 is missing a value", when it means "u0 of the initialization" which means "guess". So I think this has to be specialized.

@hersle
Copy link
Contributor Author

hersle commented Jul 18, 2024

Maybe. The stacktrace should be very indicative of the context in such a case, though.

@hersle
Copy link
Contributor Author

hersle commented Jul 19, 2024

I am doubtful that this is a good way to fix the issue, at all. It would be better to hit the old error again, instead of throwing a new error a new place. Also, get_u0() does some processing further down the pipe that extends the list of guesses from that in InitializationProblem(), so it is not really correct to throw an error at such a high level. See comment in #2873.

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.

Helpful error for missing guesses has disappeared
2 participants