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

Theory derivations should be more than simple `Sentence`s #968

Open niazim3 opened 6 years ago

niazim3 commented 6 years ago

Problem from #1356: We want to be able to place figures in theory derivations because they can aid in understanding theory derivations.

In order to do this, this original ticket proposed re-writing Derivations to rely on [Content] instead of [Sentence] (i.e., a list of arbitrary document content as opposed to [scientific] natural language). Is this the right solution? I'm not immediately sure. We should (1) figure out what things should go in derivations, (2) figure out how we actually want to build derivations, and then (3) evaluate this as a solution vs other choices.

Original description from 2018:

(Moved from #963): As mentioned in today's meeting, Derivation should be changed from https://github.com/JacquesCarette/Drasil/blob/fb0156177d785df905e0b0147b7b2b5ea57b94f9/code/drasil-lang/Language/Drasil/Chunk/Derivation.hs#L8 to type Derivation = [Contents] to eliminate the need for mkEmptyLabel here: https://github.com/JacquesCarette/Drasil/blob/fb0156177d785df905e0b0147b7b2b5ea57b94f9/code/drasil-docLang/Drasil/DocumentLanguage/Definitions.hs#L67-L76

niazim3 commented 6 years ago

I've worked on this for a while and cannot see a way around import cycles that pop up when trying to implement Derivation as a [Contents], no matter where the definition for Derivation and/or HasDerivation are placed. @samm82 @elwazana can you please take a look to see if you see a way to around the import cycles?

(Edit: this needs to be done in the addLabels branch).

JacquesCarette commented 6 years ago

I think the main problem is that

type Derivation = [Contents]

makes a Derivation into 100% a display thing -- and thus it shouldn't be a Chunk! So the right place to define it would be in the same file where Contents itself is defined.

I think it is fine if Derivation is no longer a Chunk. I don't think we ever put them into ChunkDB.

niazim3 commented 6 years ago

I tried doing that earlier, and the problem is that the HasDerivation class needs to use the Derivation definition in order to lens to it, which means it would need to import the file in which Derivation is defined (in this case). Since Document.Core uses a lot of chunks that also import Classes for definition of them as instances of a classes, there are cycles like the following image

Placing the definition for the HasDerivation class in the Document.Core file to see if it's a possible solution leads to cycles since Document.Core needs to import chunks, and since some chunks need Derivation, there are cycles like image.

Moving the class definition to Document.hs is also not an option because doing so results in the same type of cycle

image.

samm82 commented 6 years ago

Right now, there is an discrepancy in some of the HTML versions where <div id="Eqn:empty"> is being built, but the stable has <div id="">. At least for now, in the interest of getting this branch merged ASAP, could we update stable to use the built <div id="Eqn:empty">? Then we could remove the empty label hack after the merge? @JacquesCarette @szymczdm

JacquesCarette commented 6 years ago

@samm82 yes.

JacquesCarette commented 6 years ago

I think the 'right' way to fix this cycle (and other potential ones) is to change Chunk.Eq to define

data QDef r d sn = EC
          { _qua :: QuantityDict
          , _equat :: Expr
          , _ref :: r -- FIXME: to be removed
          , _deri :: d -- FIXME: to be removed
          , _refName :: sn -- FIXME: to be removed
          , _notes :: Maybe [Sentence] -- FIXME: to be removed
          }
makeLenses ''QDef

And then "later", when References, Derivation and ShortName are defined, to do

type QDefinition = QDef References Derivation ShortName

The various combinators to build things can be generalized to build QDef instead. Then only QDefinition should be exported from Language.Drasil, not QDef (though that will need to be visible internally in the drasil-lang package).

JacquesCarette commented 4 years ago

I think things have evolved on this one, which needs to be verified in more detail (by me).