Drasil icon indicating copy to clipboard operation
Drasil copied to clipboard

Generate FR(s) for constraint violation

Open samm82 opened this issue 1 year ago • 5 comments

TODO

Tasks should be pulled out to separate issues as necessary.

  • [ ] Use the existing Choice to generate the content of the FR for checking constraints https://github.com/JacquesCarette/Drasil/blob/afb6fb752b8364d2807ced7fc0c1dd6c6aba52b2/code/drasil-code/lib/Language/Drasil/Choices.hs#L38-L39
    • Investigate how to make this information available to all areas needed; SystemInformation?
  • [ ] Figure out if these verification requirements should reference the relevant tables. If so, what should be done if an input FR is NOT generated, but one for verifying input is? Should the table still be generated? What if the user manually creates an input table?
  • [ ] Split this FR into two separate ones, physical and software constraints, again based on the Choice
  • [ ] Ensure that generated code actually matches these FRs!
  • [ ] Change code generation to only raise an exception after checking all constraints, outputting all warnings to the user first
  • [ ] Add a new FR for checking output constraints

Context

Currently, information about what to do when a constraint is violated is built by hand in a requirement:

https://github.com/JacquesCarette/Drasil/blob/afb6fb752b8364d2807ced7fc0c1dd6c6aba52b2/code/drasil-example/projectile/lib/Drasil/Projectile/Requirements.hs#L31-L34

This requirement is currently not actually met by the generated program; calculations continue even with constraint violations. (I noticed this while making this manual test function for invalid inputs for my project; note the "xfail" [or "expected failure"].)

The Solution?

We have an underutilized Choice for what to do on a constraint violation (shown below) that I think we should use more to actually generate the requirement, similarly to #3259.

https://github.com/JacquesCarette/Drasil/blob/afb6fb752b8364d2807ced7fc0c1dd6c6aba52b2/code/drasil-code/lib/Language/Drasil/Choices.hs#L38-L39

Consequences

Interestingly, Projectile was written with the wrong constraint type; these should have been Exception (although I've just been focusing on the software constraints in my testing) and this likely came from me copying and modifying another example's Choices without fully understanding them 😅. Encoding this information "properly" will allow us to catch this earlier.

https://github.com/JacquesCarette/Drasil/blob/afb6fb752b8364d2807ced7fc0c1dd6c6aba52b2/code/drasil-example/projectile/lib/Drasil/Projectile/Choices.hs#L120

https://github.com/JacquesCarette/Drasil/blob/afb6fb752b8364d2807ced7fc0c1dd6c6aba52b2/code/drasil-code/lib/Language/Drasil/Choices.hs#L293-L297

We noted in #3527 that using Exception for physical constraints and Warning for software constraints is a reasonable "default" option, since physical constraints are forced by the problem itself, while software constraints merely attempt to limit the usage of the program to "reasonable" values.

Answered Questions

  1. Currently, this is what this requirement looks like for Projectile: "Verify-Input-Values: Check the entered input values to ensure that they do not exceed the data constraints. If any of the input values are out of bounds, an error message is displayed and the calculations stop." The second sentence is really what we're looking to generate here; should this be a separate requirement? (No.) If not, should we generate the first sentence as well (since it looks fairly stable) or should we allow the user some customization here? (Generate the first stable sentence.)
  2. The current requirement only checks for input constraint violations; should we also be checking for output constraint violations? (This would likely be a separate issue.) (Yes.)

samm82 avatar Jul 08 '23 06:07 samm82

I couldn't sleep so figured I'd write my thoughts down 😅 I'm planning on elaborating on this in our next meeting.

@balacij This is what I was talking to you about if you're interesting in seeing it "formalized".

samm82 avatar Jul 08 '23 06:07 samm82

Sorry you couldn't sleep @samm82. At least you took advantage of the time! :smile:

This is already a good result for you Masters @samm82. I like that you found an error in our generator, in the sense that it immediately provides motivation for your work. (If you haven't already, I strongly suggest that you start a document and add notes to remind you of things you find during your research. It can be hard to remember in the end. You might just want to build the infrastructure for your thesis and put bullet points in the sections as things come up. This particular discussion would go in the motivation section.)

Part of the problem here is that we treat our requirements as display entities only. They aren't connected to what we do. My guess is that we could delete the "requirements" from any of our examples and the code would still be generated the same. I might be wrong about that, but I think we do our generation from the instance models, but it is really the requirements that tell us what instance models are relevant. The mapping between requirements and instance models is so trivial that we've skipped the connection. As a consequence, we don't really do anything with requirements that stray from the instance models. I wonder if there are other examples of this.

For your questions I don't really understand the first one. The first and second sentence can't really be separated. "Check the entered input values to ensure that they do not exceed the data constraints. If any of the input values are out of bounds, an error message is displayed and the calculations stop." If we just say Check the values, then you haven't said what to do if there is a violation. If we just say "if any of the input values are out of bounds" we don't have the pointer to the data constraints from the first sentence.

When you are talking about generating the requirement, do you mean generating the text of the requirement, or generating the code that satisfies the requirement? From the question, I think you mean the first case. I'm fairly sure that all of our examples have something like the first sentence. If you did a survey of the examples and found some that didn't match, we could very likely change them to the exact same wording without losing anything. In that case, I think if the author of the requirements should only have to select that they want a "Verify Inputs" requirement, and the first sentence is automatically given. The variation would be in the second sentence if we wanted to distinguish between errors (stop the calculations) and warnings. We could have a parameter that let the author decide whether verify inputs results in warnings or errors. We should allow for the possibility of two requirements: one for warnings and one for errors. Each requirement would have to point to a different table of constraints (one for warnings and one for constraints).

@samm82, did I interpret your question 1 correctly?

For your second question, we should also be checking for output constraints. These can be particularly useful for metamorphic testing where we make sure that an entire suite of tests doesn't violate the constraints.

smiths avatar Jul 08 '23 14:07 smiths

Yes, you interpreted my first question correctly @smiths - I realize now that separating the two sentences doesn't really make sense.

Some other thoughts I had based on our current constraint violation behaviour:

  1. We allow for a different behaviour for physical vs. software constraints, which would likely add some complexity to this requirement (e.g., "Check the entered input values to ensure that they do not exceed the data constraints. If any of the input values are out of the physical bounds, an error message is displayed and the calculations stop. If any of the input values are out of the software bounds, a warning message is displayed and the calculations continue.")
  2. When a single violation is found, an exception is thrown immediately, even if there are other violations. I propose we output all error message before raising an exception so that the user is notified of all violations so they can fix them all at once.

samm82 avatar Jul 09 '23 18:07 samm82

For your bullet point 1 above, this is where I think you want separate requirements. One requirement for physical bounds with an error and another requirement for software constraints with a warning. To allow for more versatility, we could also provide options for physical bounds with a warning and software constraints with an error.

I like your second idea of finding all violations on the input constraints before raising an exception.

smiths avatar Jul 10 '23 13:07 smiths

To me, this is a perfect opportunity to dig and generalize. The questions that come to mind:

  • what are the different kinds of constraints that can come up?
  • where do they come from?
  • what are they attached to?
  • should they all be checked? [Some invariants would cost as much to check as running the program itself... it would be neat to find some that cost even more]
  • what should the process for checking them be? are there choices in this process?
  • when the check fails, what to do? What granularity of decision makes sense?

There's probably more, but it will be easier to find them once a stab's been give at the ones that I already list.

JacquesCarette avatar Jul 26 '23 18:07 JacquesCarette