fmidue / logic-tasks

0 stars 1 forks source link

Ausgewogenere Schwierigkeit bei Klammer-Entfernen-Aufgabe #67

Closed nimec01 closed 9 months ago

nimec01 commented 10 months ago

Ich habe mich mal an der Umsetzung eines Filters hinsichtlich #65 probiert. Anhand des minUniqueOperators Wertes in SuperfluousBracketsConfig kann man eine Mindestanzahl an Operatoren festlegen. Ich habe zwei neue Testfälle erstellt, die soweit auch erfolgreich absolviert werden.

Gibt es für mich auch eine Möglichkeit, lokal eine Aufgabe zu generieren und diese in der Konsole ausgeben zu lassen?

Des Weiteren ist mir aufgefallen, dass stack test bei mir immer wieder mal bei unterschiedlichen Testfällen hängen bleibt (auch unabhängig von meinen Änderungen). Ist das ein Phänomen, das nur bei mir auftritt?

Hier ist ein Output von einem dieser Fälle:

logic-tasks> test (suite: src-test)

Formula
  genValidBoundsClause
    should generate valid bounds [✔]
      +++ OK, passed 100 tests.     
  genValidBoundsCnf
    should generate valid bounds [✔]
      +++ OK, passed 1000 tests.
  genLiteral
    should throw an error when called with the empty list [✔]
    should generate a random literal from the given char list [✔]     
      +++ OK, passed 100 tests; 19 discarded.
  genClause
    should return the empty Clause when called with the empty list [✔]
      +++ OK, passed 100 tests.
    should return the empty Clause when called with invalid boundaries [✔]
      +++ OK, passed 100 tests; 15 discarded.
    should generate a random clause of the correct length if given valid parameters [✔]
      +++ OK, passed 100 tests.
  genCnf
    should return the empty conjunction when called with the empty list [✔]
      +++ OK, passed 100 tests.
    should generate a random cnf formula with a correct amount of clauses if given valid parameters [✔]
      +++ OK, passed 100 tests.
    should generate a random cnf formula with the correct clause length if given valid parameters [✔]     
      +++ OK, passed 100 tests.
LegalCNF
  validBoundsLegalCNF
    produces a valid config [✔]
      +++ OK, passed 1000 tests.
  invalidBoundsLegalCNF
    produces a valid config [‐]
      # PENDING: No reason given
  genIllegalSynTree
    the syntax Tree are not CNF syntax tree [✔]     
      +++ OK, passed 100 tests.
  judgeCnfSynTree
    is reasonably implemented [✔]     
      +++ OK, passed 100 tests.
  generateLegalCNFInst
    all of the formulas in the wrong serial should not be Cnf [45/100]
jvoigtlaender commented 10 months ago

Betreffs lokalem Ausprobieren, das ist hier mal knapp beschrieben und lässt sich hoffentlich anwenden: https://github.com/fmidue/logic-tasks/issues/46#issuecomment-1612003313

jvoigtlaender commented 10 months ago

... sowie hier: https://github.com/fmidue/logic-tasks/issues/48#issuecomment-1612025336

jvoigtlaender commented 10 months ago

Bzgl. des Festhängens bei Testfällen: kann sein, dass in irgendeinem der Formelgeneratoren (oder so) noch ein unter seltenen Bedingungen zuschlagender Fall von nicht abbrechender Rekursion versteckt ist. 😢

nimec01 commented 10 months ago

Mit

stack repl
> :m + Control.Monad.Output
> :m + Test.QuickCheck
> generate (generateSuperfluousBracketsInst defaultSuperfluousBracketsConfig) >>= \i -> putStrLn (stringWithSuperfluousBrackets i)

konnte ich erreichen, was ich wollte. Die Formeln enthielten bis jetzt auch immer mindestens zwei verschiedene Operatoren.

Ist die Implementation so weit in Ordnung?

jvoigtlaender commented 10 months ago

Prinzipieller Ansatz ist richtig, das Zählen ist m.E. aber noch nicht ganz korrekt, siehe oben.

jvoigtlaender commented 10 months ago

Zu überlegen wäre noch, ob nicht der Parameter minUniqueOperators, bzw. umbenannt vielleicht minUniqueBinOperators, sogar in den SynTreeConfig Teil wandern könnte (und dann Einbau der Filterung direkt in genSynTree), wodurch dann auch andere Aufgabentypen von dieser zusätzlichen Steuerungsmöglichkeit profitieren würden. (Wo diese Steuerungsmöglichkeit nicht gebraucht wird, kann der Parameter ja jeweils auf 0 oder 1 gesetzt werden.)

jvoigtlaender commented 10 months ago

Zum Zeitpunkt eines Merge müsste @owestphal mal sagen, ob es im Moment unproblematisch geht oder lieber bis nach einem eventuellen nächsten Autotool-Deploy gewartet werden sollte (weil Konfigurationstyp-Änderungen ungewünschte Auswirkungen auf im aktuellen Semester bereits gestellte Aufgabeninstanzen haben könnten).

owestphal commented 10 months ago

Änderungen an der Konfiguragtion sollten wenn möglich bis nach dem Semester warten. Sonst bekommen alle Bonusaufgaben rückwirkend neue Instanzen oder funktionieren nicht mehr. Das macht Nachvollzeihen der Fehler für die Studierenden schwierig und die Bonuspunkte als Ganzes sehr undurchsichtig.

nimec01 commented 10 months ago

Zu überlegen wäre noch, ob nicht der Parameter minUniqueOperators, bzw. umbenannt vielleicht minUniqueBinOperators, sogar in den SynTreeConfig Teil wandern könnte (und dann Einbau der Filterung direkt in genSynTree), wodurch dann auch andere Aufgabentypen von dieser zusätzlichen Steuerungsmöglichkeit profitieren würden. (Wo diese Steuerungsmöglichkeit nicht gebraucht wird, kann der Parameter ja jeweils auf 0 oder 1 gesetzt werden.)

Ich habe die Logik jetzt entsprechend verschoben. Der Standardwert von minUniqueBinOperators liegt jetzt erstmal bei 0.

jvoigtlaender commented 10 months ago

Bis auf die obigen beiden Kleinigkeiten scheint mir dies fertig.

Wegen des Einflusses auf die aktuelle Semester-Instanz muss der Merge aber warten. Wahrscheinlich sollte dann ein geeignetes Label verwendet werden, um diesen Status zu markieren.

nimec01 commented 10 months ago

Ich habe die Kleinigkeiten soeben beseitigt. Labeländerungen kann ich nicht vornehmen (falls das nochmal geändert werden soll).

jvoigtlaender commented 9 months ago

There are several too-long-lines in the PR.

nimec01 commented 9 months ago

It's fixed now.