Drasil icon indicating copy to clipboard operation
Drasil copied to clipboard

GlassBR SRS and code generator get constraints from different sources

Open jackwyand opened this issue 7 months ago • 6 comments

Spin-off of #4064 (comment)

In the GlassBR SRS the input data constraints used are from a different source than the code generator.

Creating the SRS in mkSRS, inputDataConstraints is used https://github.com/JacquesCarette/Drasil/blob/ffeefe8260399e315cad866801c7abd576e50721/code/drasil-example/glassbr/lib/Drasil/GlassBR/Body.hs#L101-L113

While in the System, constrained is used instead https://github.com/JacquesCarette/Drasil/blob/ffeefe8260399e315cad866801c7abd576e50721/code/drasil-example/glassbr/lib/Drasil/GlassBR/Body.hs#L60-L79

These two lists should be reconciled so that the SRS and code generator use constraints from the same source.

The main difference between the two at the moment is that inputDataConstraints is of type [UncertainChunk] while constrained is a [ConstrainedChunk] type. Simply using constrained in place of inputDataConstraints doesn't work because it needs the typeclass constraint HasUncertainty. I tried removing the HasUncertainty typeclass constraint from the Constraints constructor, but it is needed by a subsequent function related to creating the "Input Data Constraints" table

Constraints constructor: https://github.com/JacquesCarette/Drasil/blob/ffeefe8260399e315cad866801c7abd576e50721/code/drasil-docLang/lib/Drasil/DocDecl.hs#L77-L91

Code where typeclass constraint is needed (tracing the usage of Constraints led to here, if it is useful): https://github.com/JacquesCarette/Drasil/blob/ffeefe8260399e315cad866801c7abd576e50721/code/drasil-lang/lib/Language/Drasil/Uncertainty.hs#L53-L59

After doing some more analysis it seems like inputDataConstraints is specifically used for the "Input Data Constraints" table in the SRS, while constrained holds all of the constraints for the entire problem, such as the nominal thickness t has to be in a set of specific values, or the stress distribution factor J should be between 1 and 32. These are not mentioned in the "Input Data Constraints" table, but their constraints are in the constrained list, hence why it is used over just inputDataConstraints. Therefore, I think we need to redesign this part in order to be able to use constrained as the only constraint-containing list, and pull the necessary constraints from that list when they are needed.

One idea I had for this was to use the inputs list: https://github.com/JacquesCarette/Drasil/blob/ffeefe8260399e315cad866801c7abd576e50721/code/drasil-example/glassbr/lib/Drasil/GlassBR/Unitals.hs#L44-L46

We could use this list to determine what constraints are needed for the respective parts in the SRS. Not 100% where to start with something like that as it seems like a substantial redesign.

One question that I had while writing this is why aren't all the "Required Inputs" mentioned in the "Input Data Constraints" table in the SRS? Clearly the ones not mentioned (glass type and nominal thickness) have constraints, but are not included. Due to the current design only inputs with uncertainties are included in the "Input Data Constraints" table, going back to the original issue of not being able to mix uncertainty chunks with constrained chunks. Is this an error or is there a good reason for this? Maybe @smiths has some insight? If it is an issue, using the above solution might fix this as they would be properly recognized as inputs due to being in the inputs list.

jackwyand avatar May 21 '25 19:05 jackwyand

@jackwyand great question about the difference between the Data Constraints and the Required Inputs. The inputs that do not appear in the data constraints table are inputs that don't have constraints. $t$ and $g$ are quasi types (nice that I learned that term recently :smile:). From the specification point of view, values of $t$ and $g$ are not allowed that do not match their type. $t$ is an element of a set of values and $g$ is an element of a set of values. $g$ cannot equal "CAT" because "CAT" isn't in the set of Glass types. From the viewpoint of the specification, invalid values for $t$ and $g$ cannot be constructed. It would be a "compiler" error if a bad value arrived. The "compiler" would catch the type mismatch and generate an error message. We don't actually have a compiler that is checking the types, but that is because of our implementation choices.

If we wanted constraints, instead of quasitypes, we would make $g$ a string and have a constraint that the string has to match the given strings. This makes the SRS less abstract.

smiths avatar May 22 '25 16:05 smiths

Thanks for the clarification.

I have been having trouble coming up with ideas for only utilizing one list for all the constraints in the System that would also work with the SRS. Since the SRS requires uncertainty values from the inputs but constrained uses ConstrainedChunks and not UncertainChunks I can't think of a way to reconcile them in a nice way. I'm not really sure where to go from here with this, maybe @JacquesCarette or @balacij have some ideas?

jackwyand avatar May 22 '25 18:05 jackwyand

I don't think we should have a single list. The meaning of "input constraints" and "problem constraints" are different (even though the input constraints are a subset of the problem constraints). In fact, it might be best to break things down into multiple lists, each with a precise meaning, and then automatically build "constrained" as the union of all of those.

Then we can decide what 'uniqueness' we want. This is not a priori obvious, I think we'll need to see what it means in practice.

The core issue remains: we do need to know where all the constraints come from. Having them too scattered is a problem. This might mean changing System to contain a record of constraints of different kinds, instead of merely a list.

JacquesCarette avatar May 23 '25 14:05 JacquesCarette

I am not sure I follow with your suggestion in terms of having multiple lists, is that not what currently exists? I agree it could be cleaned up but at the moment we have multiple lists with different (albeit maybe not precise enough) names: https://github.com/JacquesCarette/Drasil/blob/ffeefe8260399e315cad866801c7abd576e50721/code/drasil-example/glassbr/lib/Drasil/GlassBR/Unitals.hs#L48-L71

https://github.com/JacquesCarette/Drasil/blob/ffeefe8260399e315cad866801c7abd576e50721/code/drasil-example/glassbr/lib/Drasil/GlassBR/Unitals.hs#L33-L35

These were reorganized slightly in ba19921 which makes it more readable as well.

jackwyand avatar May 23 '25 16:05 jackwyand

What currently exists is indeed multiple (two) lists, but whose meaning is muddled. I'm suggesting multiple lists but where each has a clearer meaning, and thus their names could then be clearer.

We should be moving (slowly!) towards two kinds of lists:

  1. human-provided ones because they contain statements of intent
  2. program-computed ones, gathered from the specification of the pieces of the system.

As we refactor pieces, we should also be moving towards that ideal goal.

JacquesCarette avatar May 23 '25 18:05 JacquesCarette

To be clear, this issue extends to the other case studies as well. Any solution that affects GlassBR should cascade to the other examples.

balacij avatar May 29 '25 19:05 balacij