Open balacij opened 3 years ago
I posted this ticket just a bit early. I will add some extra discussion (as comments) later today/tomorrow about the following:
A variant of this might be well-paired with #2873. Though, I think we can close this ticket, unless there's anything directly salvageable right now.
Revisiting this ticket, I realized that there's a better way to write "Design 2" as above, it just requires an extra language extension: ConstraintKinds.
In the example (which I've collapsed to make this post a bit easier to scroll through), we can have the main components of a ModelKind made similar to FooBar
(these ModelKinds would give us a basic set of requirements of the underlying fragments), and then we could have InstanceModels
or the Grounded Theories have an extra requirement that we should be able to generate code from them somehow (currently, primarily just being able to expose QDefinition Expr
s really).
This is a fairly nice resolution to the dubious empty typeclass I had thought of for Design 2 (which I only made do with because I didn't know we needed the extra language extension to be able to make constraint aliases).
ConstraintKinds
are indeed a nice piece of Haskell, and could be part of the solution.
At a higher level, here is how I use the Haskell parts to connect up with what we need to do:
We have different kinds of models explicitly because they contain different kinds of information, and we want to know what that information is, at least for some purposes. There are also other pieces of functionality that explicit wants to abstract over those details!
To me, this says that an ADT-based solution for ModelKinds
seems to fit best.
First, why was this type variable introduced? It was introduced to restrict "InstanceModels" to models containing Exprs (which can be totally converted into GOOL well), and the other *Models to ModelExprs (which can't be totally converted into GOOL without manual intervention).
So maybe this is where we erred. Maybe we shouldn't be trying to use Haskell's type system to enforce this? We still need to 'enforce' this, but maybe we're trying too hard?
@JacquesCarette, I like your list of how you use the different parts of Haskell (data constructors, class methods, etc.). Is this advice we should capture somewhere for future Drasil collaborators?
Probably a good idea. I'd never written that down before, nor even really thought about it consciously. But this issue pushed me to make explicit why I was feeling uncomfortable with some of the proposed solutions.
@balacij, do you know a spot in our wiki that would be good for capturing the above high level advice from @JacquesCarette? If there isn't a current spot for it, should we create a wiki page with a title like "Drasil design guidelines"?
Perhaps the Types and Typeclasses page? Right now, that page is primarily a list of types, but we can retrofit it a bit to contain the more meaningful discussion of types that @JacquesCarette wrote up for us.
Yes @balacij the Types and Typeclasses page sounds like a good home. Maybe a new heading for "Guidelines for Adding New Types and Typeclasses"? @balacij would you mind putting the information on the Types and Typeclasses page?
Yep, that sounds good me. I just posted it. @JacquesCarette's Information Encoding would probably be a good area to link to it as well, or possible moving it into entirely. I'll only add a link at the bottom of the page for now I think (but please let me know what you think!).
Looks good! Thanks @balacij.
To me, this says that an ADT-based solution for ModelKinds seems to fit best.
I thought that I understood what you had written earlier, but now I feel I'm missing something to conclude that ModelKinds
should be ADT based. I still see class methods as the solution.
So maybe this is where we erred. Maybe we shouldn't be trying to use Haskell's type system to enforce this? We still need to 'enforce' this, but maybe we're trying too hard?
This is definitely possible. We might've been tempted by something that might've turned to a dead end. Naively, we could split ModelKinds
into 2 variants; one for grounded theories, and one that's more "open" for everything else.
We should circle back on this. We need to understand the problem, so that we can settle on a design.
Understanding the existing
ModelKinds
designTaking a look at our existing ModelKinds definition: https://github.com/JacquesCarette/Drasil/blob/0ea1e82231c83ae999b58cfc871d85fbbcaa3ef3/code/drasil-theory/lib/Theory/Drasil/ModelKinds.hs#L33-L38
We have a type variable "e", which we place "Expr" or "ModelExpr" in. I believe this type variable also contributes to the issue that Dr. Carette noted -- this can certainly be confusing, but I think it also indicates that we should re-investigate #2599 and ModelKinds. The goal of this ticket is to create a new design for
ModelKinds
as a series of constraints.First, why was this type variable introduced? It was introduced to restrict "InstanceModels" to models containing Exprs (which can be totally converted into GOOL well), and the other *Models to ModelExprs (which can't be totally converted into GOOL without manual intervention).
Problems with this design (of ModelKinds + the type variable)
The type variable isn't always applicable to each model, which results in awkward fits at times.
It does indeed restrict InstanceModels to only models containing Exprs. However, I don't believe that "Expr" should be the requirement that we have to indicate usability in an instance model because not all models use "Exprs", and some won't contain any sort of expression language. DEModels are also usable in InstanceModels, but they're built using ModelExprs (in their RelationConcepts [at the moment]), which is why the smart constructors allow the users to fit them into any ModelExpr type hole: https://github.com/JacquesCarette/Drasil/blob/0ea1e82231c83ae999b58cfc871d85fbbcaa3ef3/code/drasil-theory/lib/Theory/Drasil/ModelKinds.hs#L49-L55
This indicates that the type variable isn't needed (and is irrelevant) for DEModels.
Additionally, looking at all 3 Equational* constructors, they are all forced to contain "Exprs" if they are to be used in InstanceModels, and they are all forced to contain "ModelExprs" if they are to be used in TMs/GDs.
We have implicit, unclear requirements of each ModelKind.
A lot of code is spent on passing through information for used typeclasses. However, the requirements of the ModelKinds isn't quite well documented yet, so it's difficult to say which is required for each model. https://github.com/JacquesCarette/Drasil/blob/0ea1e82231c83ae999b58cfc871d85fbbcaa3ef3/code/drasil-theory/lib/Theory/Drasil/ModelKinds.hs#L105-L116
There is a lot of code duplication.
A lot of the code duplication occurs as a result of the above point.
Adding more "ModelKinds" is difficult.
The existing ModelKinds are built using standard GADTs. Adding a new "ModelKind" requires us to create a new data constructor in drasil-theory/ModelKinds.hs, create a lens that satisfies some constraints so that it may satisfy it's lens and typeclass instances.
An Alternative Design
I've come up with 2 real designs (technically 3, but 2 are essentially the same idea w/ different stylization), with a working prototype on a stripped
drasil-lang
anddrasil-theory
on my ExprTests repo.Design 1 - using an "Abstractness" datatype to indicate whether a model is usable in code generation as an alternative to using Expr
(from: https://github.com/balacij/ExprTests/blob/614b84675be56421ec62be49ca017374ed4ff253/src/Theory/ModelKinds.hs#L52-L80 )
Note that this doesn't stop us from creating "ModelKinds Concrete (ModelExpr t)", which would be inadmissible. This is a rather brittle solution, with an unnecessary "Expr"-kind, and arbitrary "Abstractness" imposition. To make up for allowing inadmissible constructions, we are forced to force requirements in the smart constructors, e.g.:
Design 2 - describing "ModelKinds" as a typeclass with a series of typeclass constraints
We define a "CoreModelKinds" which contains the core functionality required for all *Models.
NOTE: There will need to be at least 6 constraints, but we can omit them from this proposal since they're, implementation-wise, the same as "HasUID" or "HasShortName". However, we notable replace the "Express" constraint (which would expect to convert things into ModelExprs) with a "Display" constraint which allows for things to be placed inside a new basic Display-only language (which sits atop Expr and ModelExpr, intended for formatting them next to each other [without changing them at all]).
Then we define an "Instantiable" variant (intended for things that are usable in InstanceModels):
We would need to figure out a constraint on things which would allow them to be replaced with code fragments for drasil-code to be able to generate code. I'm not entirely sure of what this constraint should be yet, however, I believe that we should work towards figuring this out so that we can implement this solution, as I believe it to be the more robust solution because this solution:
However, it's not without its cons:
Prototype code (which also has a lot of extra discussion and shows more code): https://github.com/balacij/ExprTests/blob/614b84675be56421ec62be49ca017374ed4ff253/src/Theory/ModelKinds.hs#L138-L141 https://github.com/balacij/ExprTests/blob/614b84675be56421ec62be49ca017374ed4ff253/src/Theory/ModelKinds.hs#L155-L157
Final thought
I think that "Design 2" is a potentially good solution to the problems with the existing model kinds, we would just need to figure out how to handle the "usable in code generation" constraint.
All related code can be found at: https://github.com/balacij/ExprTests