Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fortsetzung ComposeFormula & DecomposeFormula #122

Merged
merged 20 commits into from
May 10, 2024

Conversation

nimec01
Copy link
Collaborator

@nimec01 nimec01 commented Apr 2, 2024

Fügt den Aufgabentyp DecomposeFormula wie in #112 beschrieben ein. Analog zu ComposeFormula werden erstmal keine Pfeiloperatoren benutzt.

@jvoigtlaender
Copy link
Member

Dies:

Analog zu ComposeFormula werden erstmal keine Pfeiloperatoren benutzt.

verstehe ich nicht so recht. Sowohl hier als auch in ComposeFormula werden doch Pfeiloperatoren unterstützt. Bei ComposeFormula sogar explizit auch in der default config:

defaultComposeFormulaConfig :: ComposeFormulaConfig
defaultComposeFormulaConfig = ComposeFormulaConfig
{ syntaxTreeConfig = defaultSynTreeConfig { allowArrowOperators = True }

Eingeschränkt wird doch nur, dass an der Wurzel des Baumes kein <= oder => stehen darf. Anderswo dürfen diese Operatoren stehen, und an der Wurzel auch <=>.

@jvoigtlaender
Copy link
Member

Ich denke, es sollte per config checker noch geprüft werden, dass hier immer minUniqueBinOperators > 0 gesetzt ist. Und ebenso beim ComposeFormula-Typ.

@jvoigtlaender
Copy link
Member

Außerdem wird scheinbar schon beim ComposeFormula-Typ (und hier ist es dann genauso relevant) diese Bedingung aus #103 (comment) bisher nicht berücksichtigt:

  • Es wird dafür gesorgt, dass an dessen Wurzel ... die beiden darunter hängenden Teilbäume nicht gleich sind.

Das lässt sich ja mittels suchThat noch in den Generator einbringen. Dann muss allerdings auch noch ein bisschen getestet werden, ob die bisherigen config-constraints für den Aufgabentyp ausreichen. Mit sehr kleinen Setzungen etwa von maxNodes und availableAtoms könnte es ja sein, dass etwa nur Formeln der Art A /\ A möglich sind, also Verschiedenheit von linkem und rechtem Teilbaum nicht erreichbar ist.

@jvoigtlaender
Copy link
Member

(Für ComposeFormula wäre der vorige Punkt vielleicht durch Tests aufgefallen, wenn systematisch geprüft würde, ob die in completeGrade ausgegebene Musterlösung auch immer tatsächlich die Überprüfung besteht. Denn in partialGrade wäre diese dann wohl an length (nubOrd sol) /= 2 gescheitert.)

@nimec01
Copy link
Collaborator Author

nimec01 commented Apr 4, 2024

Dies:

Analog zu ComposeFormula werden erstmal keine Pfeiloperatoren benutzt.

verstehe ich nicht so recht. Sowohl hier als auch in ComposeFormula werden doch Pfeiloperatoren unterstützt. Bei ComposeFormula sogar explizit auch in der default config:

defaultComposeFormulaConfig :: ComposeFormulaConfig
defaultComposeFormulaConfig = ComposeFormulaConfig
{ syntaxTreeConfig = defaultSynTreeConfig { allowArrowOperators = True }

Eingeschränkt wird doch nur, dass an der Wurzel des Baumes kein <= oder => stehen darf. Anderswo dürfen diese Operatoren stehen, und an der Wurzel auch <=>.

Ja, ich hatte, warum auch immer, im Hinterkopf, dass wir beides ausgestellt hatten.

@jvoigtlaender
Copy link
Member

Es sollten bei der Generierung wahrscheinlich noch mehr Sonderfälle (als Gleichheit von t1 und t2) ausgeschlossen werden, damit die Aufgabe nicht manchmal "aus Versehen" gelöst werden kann.

Sei zum Beispiel bei DecomposeFormula zufällig t1 = A /\ A und t2 = B /\ B und die vorgegebene Formel: t1 \/ t2.

Dann sollen die Studierenden eigentlich (A /\ A) \/ (B /\ B) anschauen, das in die beiden Teile zerlegen und dann neu anordnen zu (B /\ B) \/ (A /\ A). Aber zufälligerweise würden sie auch die richtige Lösung erhalten, wenn sie einfach den gegebenen String umdrehen. Ohne das Verständnis der Identifizierung des Root-Symbols etc.

Das liegt hier daran, dass t1 und t2 jeweils für sich genommen spiegelsymmetrisch sind, also mirror t1 == t1 und mirror t2 == t2. Vielleicht gibt es noch ähnliche Auffälligkeiten wenn etwa mirror t1 == t2 oder so, bzw. auch beim ComposeFormula-Aufgabentyp.

Mein Vorschlag nun wäre, bei beiden Aufgabentypen sicherheitshalber zu verlangen, dass gilt: length (nubOrd [t1, t2, mirror t1, mirror t2]) == 4. Die bisherige Forderung t1 /= t2 ist damit natürlich auch schon abgedeckt.

@jvoigtlaender
Copy link
Member

For mirror to make sense in general, probably #29 should be done first.

@nimec01 nimec01 changed the title Neuer Aufgabentyp DecomposeFormula Fortsetzung ComposeFormula & DecomposeFormula Apr 12, 2024
src/LogicTasks/Syntax/DecomposeFormula.hs Outdated Show resolved Hide resolved
collectUniqueBinOpsInSynTree :: SynTree BinOp a -> [BinOp]
collectUniqueBinOpsInSynTree (Leaf _) = []
collectUniqueBinOpsInSynTree (Not x) = collectUniqueBinOpsInSynTree x
collectUniqueBinOpsInSynTree (Binary op l r) = nubOrd $ concat [
[op],
collectUniqueBinOpsInSynTree l,
collectUniqueBinOpsInSynTree r]

mirrorTree :: SynTree o c -> SynTree o c
mirrorTree (Binary b l r) = Binary b r l
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think #29 should be completed and merged first, and then this line changed to flip the directed arrows around.

@jvoigtlaender jvoigtlaender merged commit 616b40c into fmidue:master May 10, 2024
7 of 8 checks passed
@nimec01 nimec01 deleted the decompose-task branch September 3, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants