JacquesCarette
Order of reading inputs, deriving pseudo-input values, and checking constraints is incorrect
Spin-off of #4064 (comment)
Examining the flow of the generated code it is currently: https://github.com/JacquesCarette/Drasil/blob/479e5a6260af37ad2a1db5e319e51f791eff5f37/code/stable/glassbr/src/java/GlassBR/InputParameters.java#L37-L52
- Read inputs
- Calculate derived values
- Check all of the constraints
This is a problem because some of the derived values could depend on the inputs being valid in the first place. At the very least the constraint checking should be split into two phases: input constraints and derived input constraints. These would take place after the respective inputs are determined.
Building on this, the code should also be commented to specify Physical Constraints vs. Software Constraints. For example, some variables are checked in two separate places:
Software constraint: https://github.com/JacquesCarette/Drasil/blob/479e5a6260af37ad2a1db5e319e51f791eff5f37/code/stable/glassbr/src/python/InputParameters.py#L254-L262
Physical constraint: https://github.com/JacquesCarette/Drasil/blob/479e5a6260af37ad2a1db5e319e51f791eff5f37/code/stable/glassbr/src/python/InputParameters.py#L326-L333
These constraints should also be checked in the correct order (which they are in this case), where software constraints take precedence over physical constraints. @balacij mentioned in the original comment that this may already be done, however it should be checked if it is manually done or automatically sorted.
Hmm, why software constraints first? Are physics constraints even more fundamental?
It would also be nice if the error report was more informative as to the the reason (i.e. mention Software or Physics, and the origin of the constraint) as well as having a more informative Exception message.
If I remember correctly: software constraints constrain inputs to regions where the software algorithm is applicable, and the physical constraints constrain inputs to only reasonable values. I was assuming that (1) the software constraints would guard the rest of the code from out of bounds exceptions and such and (2) physical constraints could be placed on derived inputs as well. @smiths Am I getting this right? What are your thoughts on this?
The physical constraint are to inputs that "make sense" physically. I believe that the software constraints are those where we know the computed results will have some degree of accuracy.
As I say this, and we seem to be somewhat confused on this, we should
- document what we mean thoroughly
- think through the differents kinds of constraints there might be, and implement them.
I think this is exactly the 'frame' problem!
It would also be nice if the error report was more informative as to the the reason (i.e. mention Software or Physics, and the origin of the constraint) as well as having a more informative Exception message.
Sorry about that, I was probably rushing while writing this (end of day Friday before the long weekend :) ). I plan on looking into it more and can add more to this issue as I go.
@JacquesCarette is correct. The physical constraints are things that have to be true due to physics. It is physically impossible for a length to be less than zero. It is physically impossible for Poisson's ratio to be greater than 0.5. The software constraints are less fundamental. They are the practical constraints. The height of the solar water heating tank should have a reasonable value. A tank with a height of 500 m does not make sense next to a residential home. There isn't a physical reason that such a high tank isn't allowed, but for practical reasons, we add the software constraints.
Put another way, the physical constraints are necessary. They are not optional. The software constraints are optional.
Thank you, @smiths. Since we're meeting soon, I've just added a meeting agenda item if we have time to discuss.
Also, regarding the "constraint checking order," I had a brief conversation with Reed and we agreed that it doesn't matter much outside of the dependency chain/graph. We should probably be ordering about that first.