JacquesCarette / Drasil

Generate all the things (focusing on research software)
https://jacquescarette.github.io/Drasil
BSD 2-Clause "Simplified" License
143 stars 26 forks source link

Reasonable value improvements #1808

Open bmaclach opened 5 years ago

bmaclach commented 5 years ago

Currently reasonable values are introduced at the same level as constraints in the chunk hierarchy (ConstrainedChunk and ConstrConcept). Reasonable values should instead be introduced independently from constraints. New chunk types should be created to lie between QuantityDict and ConstrainedChunk and between DefinedQuantityDict and ConstrConcept. These new chunk types would add the field for reasonable values, and then ConstrainedChunk and ConstrConcept should be updated to remove their reasonable value fields.

Also, the current HasReasVal class provides function reasVal which is a lens to a Maybe Expr. This should instead be just a lens to Expr, and the new chunks described above should represent the reasonable value as Expr, not Maybe Expr. Then a new class, MayHaveReasVal, should be created and provide a lens to a Maybe Expr, since some things will not have reasonable values.

JacquesCarette commented 4 years ago

Have any updates on this issue been made?

bmaclach commented 4 years ago

No updates, the original description is still accurate.

Awurama-N commented 3 years ago

I started working on this ticket by looking for where everything was defined to gain some more understanding on the "Chunk hierarchy" and "chunk types".

I still have a few questions though:

@JacquesCarette @smiths

JacquesCarette commented 3 years ago

Your summary of the hierarchy and some of the types accords with my memory of them.

Awurama-N commented 3 years ago

Okay thank you!, so the new chunk type created that lies between ConstrainedChunk and QuantityDict should add one of the new features that ConstrainedChunk adds to QuantityDict instead. So it would be ConstrainedChunk -> NewChunk -> QuantityDict & DefinedQuantityDict -> NewChunk2 -> ConstrConcept?

@JacquesCarette

JacquesCarette commented 3 years ago

So there should be ConstrainedChunk -> NewChunk -> QuantityDict and ConstrainedChunk -> NewChunk2 -> QuantityDict.

There might be a need to do something similar with DefinedQuantityDict and ConstrConcept, I'm less clear about that.

Awurama-N commented 3 years ago

I've been going through some chunk types and wanted to share what i think I should do... currently this is how ConstrainedChunk & QuantityDict are defined:

data ConstrainedChunk = ConstrainedChunk { _qd :: QuantityDict
                                         , _constr :: [Constraint]
                                         , _reasV :: Maybe Expr
                                         }
makeLenses ''ConstrainedChunk
data QuantityDict = QD { _id' :: IdeaDict
                       , _typ' :: Space
                       , _symb' :: Stage -> Symbol
                       , _unit' :: Maybe UnitDefn
                       }
makeLenses ''QuantityDict

i'll have to create a new file New.hs and have ConstrainedChunk now be:

data ConstrainedChunk = ConstrainedChunk { _nc :: NewChunk
                                         , _constr :: [Constraint]
                                         , _reasV :: Maybe Expr
                                         }
makeLenses ''ConstrainedChunk
data NewChunk = NewChunk (or NC) { _qd :: QuantityDict
                                 ****
                                 ****
                                 }

The questions i have are?: what should the new chunk type should be called? where the **** is, what should the contents of this chunk type be aside QuantityDict? what classes should it initiate in its file? (instead of ConstrainedChunk) is it really necessary to create a new file? or should i put it in an already existing file, as long as the sequence doesn't break.

@JacquesCarette

Awurama-N commented 3 years ago

@JacquesCarette Also can i first go ahead and change the function reasVal so its a lens to Expr instead of Maybe Expr?

JacquesCarette commented 3 years ago

Note: you don't need to tag me, I'll get an email either way.

This process is good (i.e. listing out your knowledge and questions) in issues.

So what you want to end up with is essentially:

data ConstrainedQDef = ConstrainedQDef { _qd :: QuantityDict
                                                                  , _constr :: [Constraint] -- should really be NEList
                                                                  }
-- not the lack of Maybe below
data ReasonableValueQDef = RVQD { _qd :: QuantityDict
                                                        , reasV :: Expr
                                                        }
data ConstrReasQDef = CRQD { _qd :: QuantityDict
                                                , _constr :: [Constraint]
                                                , reasV :: Expr
                                                }

and then actually delete ConstrainedChunk entirely, once you are done migrating the examples.

For now, you can put all of them in the same file as with ConstrainedChunk, we can split them later.

You can't actually change _reasV in ConstrainedChunk to be a lens to Expr because too many examples don't actually have a 'reasonable value' defined in them. That's why you need to create ConstrainedQDef first, switch the constructors of ConstrainedChunk that put Nothing in _reasV to instead build ConstrainedQDef (and fix whatever breaks). Then you can make that change -- and you may as well change the name to ConstrReasQDef while doing it.

Awurama-N commented 3 years ago

A question: in going through & editing the code I reached a point were i had to look through were the constructors (cuc, cvc, cnstrw) that previously were for ConstrainedChunk are used. At a point i saw an instance of when cnstrw is used but then leads to cvc being used when you follow the link to the list item:

constrained :: [ConstrainedChunk]
constrained = map cnstrw inputDataConstraints ++ 
  [cnstrw probBr, cnstrw probFail, cnstrw stressDistFac] 

when the link to probFail is followed it leads to:

probFail = cvc "probFail" (nounPhraseSP "probability of failure")
  (sub cP lFail) Rational
  [probConstr] (Just $ dbl 0.4)

Is this normal? It seems like the usage of a constructor within another that was defined in the same file and originally have the same chunk type.

JacquesCarette commented 3 years ago

This is harmless. I think someone was just trying to be uniform when defining the constrained list. In this case, the extra cnstrw on probFail could be removed, because it's already of the right type.

balacij commented 1 year ago

While this ticket has a design, the ticket doesn't explain why 'reasonable values' are needed, where they should propagate to, nor the choices made in the design. Why do we need 'reasonable values' and where do they appear? Why should reasonable values be defined on the quantity-chunk-level and not on the SRS-problem-level?

JacquesCarette commented 1 year ago

Those are good questions @balacij . Having 'reasonable values' the way they exist in Drasil (aka at the Chunk level) is actually a non-trivial innovation we've done [that we need to document in a paper somewhere...] but just isn't documented anywhere. I guess it might show up in @szymczdm 's thesis.

This is yet another issue that links in to the design of the "knowledge capture" part.

smiths commented 1 year ago

@balacij reasonable values are part of the requirements documentation. They give an idea of a typical value that helps the reader understand the problem. The goal is to have the documentation capture expert knowledge. We saw the value of "reasonable values" in your beam bending example. Some of your initial testing results were strange because you had a reasonable Young's modulus (of 200 GPa), but an unreasonable load (of 1 N). Although it is mathematically well defined, asking the deflection of a steel beam under the weight of an apple.

I don't know if we currently do this, but the reasonable values are problem specific. It is the combination of values that are reasonable, not a single value. For instance, a load of 1 N would be reasonable if the software was used for a school project where students built a structure using popsicle sticks.

balacij commented 1 year ago

Thank you both!

I don't know if we currently do this, but the reasonable values are problem specific. It is the combination of values that are reasonable, not a single value. For instance, a load of 1 N would be reasonable if the software was used for a school project where students built a structure using popsicle sticks.

Reasonable values are captured through ConstraintChunks at the moment -- ConstrainedChunks are QuantityDicts with constraints and (optional) 'reasonable values', so it's at the quantity-level as opposed to problem level. I like this line of thinking you bring up. I don't think reasonable values should be tied at the level of definition of quantities (e.g., the QuantityDict referred to in a ConstrainedChunk should have a different UID than the CC).

Should the Physical System Description introduce all the input symbols and output symbols of a problem? If so, we should be able to search for the input symbols and output symbols in the user-authored PSDs rather than designation in the SystemInformation packets. Additionally, if this is the case, then we might also consider discussing the reasonable values there too (this might also draw up a sort of rationale for the reasonable values declared in the table of input symbols).

smiths commented 1 year ago

I like the idea of providing a rationale for a given set of reasonable values. My view is that reasonable values are pieces of knowledge that are tied to a specific problem. I don't have an opinion on where the knowledge is stored inside Drasil, but for the generated SRS, it could appear in the introduction, in the physical system description, and in the table summarizing the input constraints (data constraints table).

If I remember correctly, I've talked about this recently with @samm82 in the context of Projectile. For Projectile there are typical problems, which each have typical values, such as:

I could make a similar list of potential typical problems for the beam bending problem, or ssp, or any of the examples.

The reasonable values are another example of a program family variability. We have a choice when we create a specific family member. Will we allow a range of typical values, or will we customize our problem to just a few options? For instance, we could say the purpose of the software is to support high school physics experiments, so the typical values wouldn't include firing bullets.

In the introduction to the SRS we can mention the typical examples that are in the scope of the project. If we stored this information as chunks, we could generate this text. The projectile introduction already mentions some typical examples, but those examples aren't tied to specific sets of typical (for that example) input values.

JacquesCarette commented 1 year ago

In a way, this is related to #2306 : what is the intent of reasonable values? I think the exposition by @smiths above (indirectly) shows that our current implementation does not match the intent.