Design needed for relToQD and RelationConcept implementation

[niazim3]

In issue #549, the errors for certain types that are instance of HasShortName have been removed. However, as of fdf90d9, RelationConcept now has an error thrown in relation to HasShortName.

This was done because

Drasil/code/Example/Drasil/GlassBR/Body.hs

Lines 146 to 148 in fdf90d9

	_definitions = dataDefns ++
	(map (relToQD gbSymbMap) iModels {-[RelationConcept]-}) ++
	(map (relToQD gbSymbMap) tModels {-[RelationConcept]-}) ++

imodels and tmodels are lists of RelationConcepts and trying to extract shortnames from there led to deduction errors.

It was originally thought that either RelationConcept should have a shortname OR imodels and tmodels should not be [RelationConcept] / should be named differently.

@JacquesCarette mentioned that RelationConcept is too 'raw' to be the only material for creating models and that perhaps it should not be augmented "as it has other flaws, so I'm not so keen on relying on it even more".

relToQD is a horrible hack too.

"I get the feeling that this is another piece that needs just a bit more design - we need to understand what these various data-structures are used for, i.e. what actual knowledge is being encoded here. To understand if the encoding is adequate. I get the feeling it's not, so that anything local patch we apply now would be a mere band-aid that would need to be fixed again soon."

I'm unsure who to assign this issue to, so I'm assigning @JacquesCarette and myself for now...

[JacquesCarette]

I think this issue is going to be important to fix to deal with both #695 and #891 properly.

Roughly speaking, neither instance models nor theory models should use RelationConcept at all. Really they should use some type like QDefinition or a superset of it. [Eventually, we'll have other kinds of definitions than just equalities, but I'm not sure where they are now.]

[JacquesCarette]

This is now half fixed: instance models use a more refined definition that differentiate between equational models, DE models and 'other' models. That type needs to pulled out of there so that theory models can use this too.

Note that relToQD itself is already completely gone. RelationConcept will stay for quite a while yet, as those models that use it (i.e. DE and other) will stay like that right now.

[JacquesCarette]

Just an update: this is partly done, but than it was when I last looked too, but still is not finished.

[JacquesCarette]

Hmm, the fixes were on a branch that was never pulled in. relToQD is not gone at all.

[balacij]

It looks like we only use relToQD in GlassBR (for converting IMods) and Projectile (for converting IMods + GenDefns). In other words, once #2493 and #2388 are complete, we'll be able to remove it completely I believe.

[JacquesCarette]

Hurray!

[balacij]

Just an update, relToQD is removed. RelationConcepts are still currently used as discussed earlier (in ModelKinds/DEModels for example).

I think that we will need to spend a significant amount of time to fully get rid of RelationConcepts (if we want to do that) since they're a "catch all" general case.

Remaining usage of RelationConcept appears to be primarily in drasil-example, so I think we'll even be able to remove most/all of it's usage when we fully convert them using #2371.

drasil-docLang/Drasil/DocumentLanguage/Definitions.hs
51:-- and a 'RelationConcept' (called automatically by 'SCSSub' program).

drasil-example/Drasil/PDController/IModel.hs
35:imPDRC :: RelationConcept

drasil-example/Drasil/PDController/GenDefs.hs
30:gdPowerPlantRC :: RelationConcept

drasil-example/Drasil/GamePhysics/IMods.hs
138:col2DRC :: RelationConcept

drasil-example/Drasil/GamePhysics/GDefs.hs
18:genDefs :: [RelationConcept]
21:impulseGDef :: RelationConcept
42:conservationOfMomentGDef :: RelationConcept
79:accelerationDueToGravityGDef :: RelationConcept
120:relativeVelocityInCollisionsGDef :: RelationConcept
139:coefficientOfRestitutionGDef :: RelationConcept
160:torqueGDef :: RelationConcept
177:momentOfInertiaGDef :: RelationConcept

drasil-example/Drasil/PDController/TModel.hs
34:laplaceRC :: RelationConcept
66:invlaplaceRC :: RelationConcept
95:tmSOSystemRC :: RelationConcept

drasil-example/Drasil/GamePhysics/GenDefs.hs
26:{-conservationOfMomentGDef :: RelationConcept

drasil-example/Drasil/NoPCM/IMods.hs
44:eBalanceOnWtrRC :: RelationConcept

drasil-example/Drasil/SSP/IMods.hs
468:nrmShrForNumRC :: RelationConcept
505:nrmShrForDenRC :: RelationConcept
536:intsliceFsRC :: RelationConcept

drasil-example/Drasil/SSP/GenDefs.hs
84:normForcEq :: RelationConcept
104:bsShrFEq :: RelationConcept
127:resShr :: RelationConcept
153:mobShr :: RelationConcept
172:effNormF :: RelationConcept
207:resShearWO :: RelationConcept
230:mobShearWO :: RelationConcept
253:momentEql :: RelationConcept
455:sliceWght :: RelationConcept
588:baseWtrF :: RelationConcept
671:srfWtrF :: RelationConcept

drasil-example/Drasil/DblPendulum/IMods.hs
37:angularDisplacementRC :: RelationConcept

drasil-example/Drasil/SWHS/GenDefs.hs
31:--since referencing implementation for RelationConcept hasn't--
32:--stabilized yet (since RelationConcept isn't an instance of --
43:rocTempSimpRC :: RelationConcept

drasil-example/Drasil/SWHS/IMods.hs
51:eBalanceOnWtrRC :: RelationConcept
197:eBalanceOnPCMRC :: RelationConcept
324:heatEInWtrRC :: RelationConcept
353:heatEInPCMRC :: RelationConcept

drasil-example/Drasil/SWHS/TMods.hs
136:latentHtERC :: RelationConcept
171:nwtnCoolingRC :: RelationConcept

drasil-lang/Language/Drasil.hs
   <SNIP>

drasil-theory/Theory/Drasil/ModelKinds.hs
   <SNIP>

drasil-lang/Language/Drasil/Chunk/Relation.hs
   <SNIP>

[balacij]

I don't think there's anything left to do in this ticket because we've removed relToQd, and we're working on removing as many RelationConcepts as possible (tracked in #2371). I don't think we need to necessarily remove RelationConcept at all, it's useful for display purposes in some niche cases. Worst case, I don't think it's harmful.

Shall we close this ticket in favour of #2371 @JacquesCarette ?

[JacquesCarette]

Agreed.