idaes-pse
idaes-pse copied to clipboard
[RFC] Have initialize methods return final solver status
It came up today that we currently have no method of confirming solver status a the end of initialization routines. For one, this limits us when writing tests for these routines, as we have no way to check that they actually solved correctly.
One solution to this would be to have initialize methods return the final solver status at the end, which we could then checks as normal. However, this would be an API change and would require many models to be updated to handle the additional output.
Currently, property and control volume initialize methods return a single dict (flags indicating which state variables were left fixed and need to be unfixed later). Adding an additional output would require:
- Updating all models which use assign the returned values to accept two values instead of 1.
- Having a default value for the
flagsreturn, and returning it in all cases. Currentlyinitializemethods generally returnNoneif no state vars are left fixed. - Making sure none of the above breaks anything else.
So, this is not a trivial thing that can be slipped in without anyone noticing, and this would require a bit of tedium to update everything.
Update:
Have started adding new InitializaitonErrors to all models. To Do:
- [x] core property packages #618
- [x] generic unit models #627
- [ ] distillation column models
- [ ] power plant models
- [ ] gas-solids contactor models
As a less disruptive alternative, we could add and initialized flag to the models. Maybe in the ProcessBlockData class. Give it an enumerated type like NOT_SUPPORTED, NOT_RUN, SUCCESS, FAIL. Not supported could could be the default, in case some models haven't done anything with it yet. It won't interfere with the initialize methods that already return stuff for other purposes.
Commenting on this PR just to mention that I would appreciate this feature as well. Will think more about if there are any clever ways of implementing it.
@eslickj Whilst attaching the initialization status to the model would avoid the API issues, I don't really like the idea attaching information like that to the model. For one, it will become out of date as soon as the next solver call is made - i.e., it only tells us that the model was initialized at some point, but not when.
Another possibility would be to shoe-horn the solver status into the existing flags, but that would involve mixing two different types of information into one dict (doable, but maybe not good practice).
@andrewlee94, this is a really unpleasant suggestion, but I've wondering about using @Robbybp's context manager that allows you to temporarily fix certain variables or get submodels for initialization. I know it would be a huge pain to update things, but if we would break the API for this, we could consider lumping those together.
Noting outcome of discussion at Dev Team call: it was decided to have initialization routines raise an Exception if the final step fails. Initialization routines could optionally raise an Exception earlier if desired.
@andrewlee94 any update on this?
This is an on going discussion. A few of us think it is a good idea, but it needs a lot of planning.
This has been rolled into the new initialization API: #1051