From 6dbc3ee909044b731ab27d0e4d8c4b2397fd5b96 Mon Sep 17 00:00:00 2001 From: Sol Dubock <94075844+sjd210@users.noreply.github.com> Date: Mon, 30 Sep 2024 16:54:46 +0100 Subject: [PATCH 1/4] Add allowScalingCoefficients option --- .../dos/IsaacSymbolicChemistryQuestion.java | 17 +++++++++++++++-- .../dto/IsaacSymbolicChemistryQuestionDTO.java | 17 +++++++++++++++-- .../quiz/IsaacSymbolicChemistryValidator.java | 2 ++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacSymbolicChemistryQuestion.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacSymbolicChemistryQuestion.java index 8f6256532..fb7ca50d6 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacSymbolicChemistryQuestion.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacSymbolicChemistryQuestion.java @@ -19,7 +19,7 @@ import uk.ac.cam.cl.dtg.isaac.dos.content.DTOMapping; import uk.ac.cam.cl.dtg.isaac.dos.content.JsonContentType; import uk.ac.cam.cl.dtg.isaac.dto.IsaacSymbolicChemistryQuestionDTO; -import uk.ac.cam.cl.dtg.isaac.quiz.IsaacOldSymbolicChemistryValidator; +import uk.ac.cam.cl.dtg.isaac.quiz.IsaacSymbolicChemistryValidator; import uk.ac.cam.cl.dtg.isaac.quiz.ValidatesWith; /** @@ -27,11 +27,12 @@ */ @DTOMapping(IsaacSymbolicChemistryQuestionDTO.class) @JsonContentType("isaacSymbolicChemistryQuestion") -@ValidatesWith(IsaacOldSymbolicChemistryValidator.class) +@ValidatesWith(IsaacSymbolicChemistryValidator.class) public class IsaacSymbolicChemistryQuestion extends IsaacSymbolicQuestion { @JsonProperty("isNuclear") private boolean isNuclear; private boolean allowPermutations; + private boolean allowScalingCoefficients; /** * @return whether the question is a nuclear question or not @@ -58,4 +59,16 @@ public void setNuclear(boolean nuclear) { public void setAllowPermutations(boolean allowPermutations) { this.allowPermutations = allowPermutations; } + + /** + * @return whether the question allows coefficients to be multiplied e.g. 10 H2 + 5 O2 -> 10 H2O + */ + public boolean getAllowScalingCoefficients() { return allowScalingCoefficients; } + + /** + * @param allowScalingCoefficients set whether the question allows coefficients to be multiplied e.g. 10 H2 + 5 O2 -> 10 H2O + */ + public void setAllowScalingCoefficients(boolean allowScalingCoefficients) { + this.allowScalingCoefficients = allowScalingCoefficients; + } } diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/IsaacSymbolicChemistryQuestionDTO.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/IsaacSymbolicChemistryQuestionDTO.java index 683210579..0903227d3 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/IsaacSymbolicChemistryQuestionDTO.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/IsaacSymbolicChemistryQuestionDTO.java @@ -17,15 +17,16 @@ import com.fasterxml.jackson.annotation.JsonProperty; import uk.ac.cam.cl.dtg.isaac.dos.content.JsonContentType; -import uk.ac.cam.cl.dtg.isaac.quiz.IsaacOldSymbolicChemistryValidator; +import uk.ac.cam.cl.dtg.isaac.quiz.IsaacSymbolicChemistryValidator; import uk.ac.cam.cl.dtg.isaac.quiz.ValidatesWith; @JsonContentType("isaacSymbolicChemistryQuestion") -@ValidatesWith(IsaacOldSymbolicChemistryValidator.class) +@ValidatesWith(IsaacSymbolicChemistryValidator.class) public class IsaacSymbolicChemistryQuestionDTO extends IsaacSymbolicQuestionDTO { @JsonProperty("isNuclear") private boolean isNuclear; private boolean allowPermutations; + private boolean allowScalingCoefficients; /** * @return whether the question is a nuclear question or not @@ -52,4 +53,16 @@ public void setNuclear(boolean nuclear) { public void setAllowPermutations(boolean allowPermutations) { this.allowPermutations = allowPermutations; } + + /** + * @return whether the question allows coefficients to be multiplied e.g. 10 H2 + 5 O2 -> 10 H2O + */ + public boolean getAllowScalingCoefficients() { return allowScalingCoefficients; } + + /** + * @param allowScalingCoefficients set whether the question allows coefficients to be multiplied e.g. 10 H2 + 5 O2 -> 10 H2O + */ + public void setAllowScalingCoefficients(boolean allowScalingCoefficients) { + this.allowScalingCoefficients = allowScalingCoefficients; + } } diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacSymbolicChemistryValidator.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacSymbolicChemistryValidator.java index 87b6eafd7..2ffc68945 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacSymbolicChemistryValidator.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacSymbolicChemistryValidator.java @@ -192,6 +192,8 @@ public QuestionValidationResponse validateQuestionResponse(final Question questi req.put("test", submittedFormula.getMhchemExpression()); req.put("description", chemistryQuestion.getId()); req.put("allowPermutations", String.valueOf(chemistryQuestion.getAllowPermutations())); + req.put("allowScalingCoefficients", String.valueOf(chemistryQuestion.getAllowScalingCoefficients())); + req.put("questionID", question.getId()); if (chemistryQuestion.isNuclear()) { From 1ce6488df92f2590cc45b63c40d5b399aa5ddf04 Mon Sep 17 00:00:00 2001 From: Sol Dubock <94075844+sjd210@users.noreply.github.com> Date: Fri, 4 Oct 2024 13:46:13 +0100 Subject: [PATCH 2/4] Remove incidental empty line --- .../cam/cl/dtg/isaac/quiz/IsaacSymbolicChemistryValidator.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacSymbolicChemistryValidator.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacSymbolicChemistryValidator.java index 2ffc68945..b8a7ce240 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacSymbolicChemistryValidator.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacSymbolicChemistryValidator.java @@ -193,7 +193,6 @@ public QuestionValidationResponse validateQuestionResponse(final Question questi req.put("description", chemistryQuestion.getId()); req.put("allowPermutations", String.valueOf(chemistryQuestion.getAllowPermutations())); req.put("allowScalingCoefficients", String.valueOf(chemistryQuestion.getAllowScalingCoefficients())); - req.put("questionID", question.getId()); if (chemistryQuestion.isNuclear()) { From f005f973bcb26f934ba868606408747b9809036c Mon Sep 17 00:00:00 2001 From: Sol Dubock <94075844+sjd210@users.noreply.github.com> Date: Fri, 4 Oct 2024 13:49:13 +0100 Subject: [PATCH 3/4] Reset to old chemistry validator (for now) --- .../cam/cl/dtg/isaac/dos/IsaacSymbolicChemistryQuestion.java | 4 ++-- .../cl/dtg/isaac/dto/IsaacSymbolicChemistryQuestionDTO.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacSymbolicChemistryQuestion.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacSymbolicChemistryQuestion.java index fb7ca50d6..ca26b9bc8 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacSymbolicChemistryQuestion.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacSymbolicChemistryQuestion.java @@ -19,7 +19,7 @@ import uk.ac.cam.cl.dtg.isaac.dos.content.DTOMapping; import uk.ac.cam.cl.dtg.isaac.dos.content.JsonContentType; import uk.ac.cam.cl.dtg.isaac.dto.IsaacSymbolicChemistryQuestionDTO; -import uk.ac.cam.cl.dtg.isaac.quiz.IsaacSymbolicChemistryValidator; +import uk.ac.cam.cl.dtg.isaac.quiz.IsaacOldSymbolicChemistryValidator; import uk.ac.cam.cl.dtg.isaac.quiz.ValidatesWith; /** @@ -27,7 +27,7 @@ */ @DTOMapping(IsaacSymbolicChemistryQuestionDTO.class) @JsonContentType("isaacSymbolicChemistryQuestion") -@ValidatesWith(IsaacSymbolicChemistryValidator.class) +@ValidatesWith(IsaacOldSymbolicChemistryValidator.class) public class IsaacSymbolicChemistryQuestion extends IsaacSymbolicQuestion { @JsonProperty("isNuclear") private boolean isNuclear; diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/IsaacSymbolicChemistryQuestionDTO.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/IsaacSymbolicChemistryQuestionDTO.java index 0903227d3..47cc568c7 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/IsaacSymbolicChemistryQuestionDTO.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/IsaacSymbolicChemistryQuestionDTO.java @@ -17,11 +17,11 @@ import com.fasterxml.jackson.annotation.JsonProperty; import uk.ac.cam.cl.dtg.isaac.dos.content.JsonContentType; -import uk.ac.cam.cl.dtg.isaac.quiz.IsaacSymbolicChemistryValidator; +import uk.ac.cam.cl.dtg.isaac.quiz.IsaacOldSymbolicChemistryValidator; import uk.ac.cam.cl.dtg.isaac.quiz.ValidatesWith; @JsonContentType("isaacSymbolicChemistryQuestion") -@ValidatesWith(IsaacSymbolicChemistryValidator.class) +@ValidatesWith(IsaacOldSymbolicChemistryValidator.class) public class IsaacSymbolicChemistryQuestionDTO extends IsaacSymbolicQuestionDTO { @JsonProperty("isNuclear") private boolean isNuclear; From d5161b39ac972fe8831e681d695ce92a516dae2e Mon Sep 17 00:00:00 2001 From: Sol Dubock <94075844+sjd210@users.noreply.github.com> Date: Wed, 23 Oct 2024 09:49:08 +0100 Subject: [PATCH 4/4] Allow approved error messages to show front-end --- .../quiz/IsaacSymbolicChemistryValidator.java | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacSymbolicChemistryValidator.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacSymbolicChemistryValidator.java index b8a7ce240..2835964d7 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacSymbolicChemistryValidator.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacSymbolicChemistryValidator.java @@ -30,6 +30,7 @@ import java.util.HashMap; import java.util.List; import java.util.Objects; +import java.util.Set; import static uk.ac.cam.cl.dtg.isaac.api.Constants.*; @@ -55,6 +56,11 @@ private enum MatchType { private final String chemistryValidatorUrl; private final String nuclearValidatorUrl; + private final Set VALID_ERROR_FEEDBACK = Set.of( + "Division by zero is undefined!", + "Your answer contains invalid syntax!" + ); + public IsaacSymbolicChemistryValidator(final String hostname, final String port) { this.nuclearValidatorUrl = "http://" + hostname + ":" + port + "/nuclear/check"; this.chemistryValidatorUrl = "http://" + hostname + ":" + port + "/chemistry/check"; @@ -213,9 +219,9 @@ public QuestionValidationResponse validateQuestionResponse(final Question questi + "\" with symbolic chemistry checker: " + response.get("error")); } + closestMatch = formulaChoice; + closestResponse = response; containsError = true; - // If parsing was unsuccessful the student provided the wrong type - isNuclear = !chemistryQuestion.isNuclear(); break; } @@ -325,9 +331,12 @@ public QuestionValidationResponse validateQuestionResponse(final Question questi if (containsError) { // User input contains error terms. - // FIXME: This currently clashes with determining whether the submitted answer was the wrong type - // Inequality should be changed to not allow Nuclear syntax in Chemistry questions and vice versa - feedback = new Content("Your answer contains invalid syntax!"); + if (closestResponse != null && VALID_ERROR_FEEDBACK.contains((String) closestResponse.get("error"))) { + feedback = new Content((String) closestResponse.get("error")); + } else { + // Default error message + feedback = new Content("Your answer contains invalid syntax!"); + } } else if (closestMatch != null && closestMatchType == MatchType.EXACT) {