diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java index 548972a21e..7c13167e88 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java @@ -45,6 +45,9 @@ public final QuestionValidationResponse validateQuestionResponse(final Question IsaacCoordinateQuestion coordinateQuestion = (IsaacCoordinateQuestion) question; CoordinateChoice submittedChoice = (CoordinateChoice) answer; + Integer sigFigsMin = coordinateQuestion.getSignificantFiguresMin(); + Integer sigFigsMax = coordinateQuestion.getSignificantFiguresMax(); + // STEP 0: Is it even possible to answer this question? if (null == coordinateQuestion.getChoices() || coordinateQuestion.getChoices().isEmpty()) { @@ -127,10 +130,6 @@ public final QuestionValidationResponse validateQuestionResponse(final Question } List choiceItems = coordinateChoice.getItems().stream().map(i -> (CoordinateItem) i).collect(Collectors.toList()); - // Get significant figures to validate with - Integer sigFigsMin = coordinateQuestion.getSignificantFiguresMin(); - Integer sigFigsMax = coordinateQuestion.getSignificantFiguresMax(); - // Check that the items in the submitted answer match the items in the choice numerically boolean allItemsMatch = false; @@ -151,10 +150,10 @@ public final QuestionValidationResponse validateQuestionResponse(final Question CoordinateItem choiceItem = choiceItems.get(coordIndex); CoordinateItem submittedItem = submittedItems.get(coordIndex); // Check that the submitted item matches the choice item - if (!coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax)) { + if (!coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, false)) { allItemsMatch = false; // Check if this is just a significant figures mismatch - if (!coordinateItemsMatch(submittedItem, choiceItem, null, null)) { + if (!coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, true)) { allItemsMatchWithoutSigFigs = false; // Exit early on mismatch: break; @@ -169,7 +168,8 @@ public final QuestionValidationResponse validateQuestionResponse(final Question break; } else if (allItemsMatchWithoutSigFigs) { feedback = new Content(DEFAULT_VALIDATION_RESPONSE); - feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs"))); + // Too many sig figs, or too few sig figs but the choice has trailing zeros; otherwise correct + feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs", "sig_figs_too_many"))); break; } @@ -179,23 +179,37 @@ public final QuestionValidationResponse validateQuestionResponse(final Question // For correct choices, check if the submitted items are a proper subset of the choice if (coordinateChoice.isCorrect() && (choiceItems.size() > submittedItems.size())) { boolean allSubmittedItemsInChoiceItems = true; + boolean allItemsInChoiceWithoutSigFigs = true; for (CoordinateItem submittedItem : submittedItems) { boolean submittedItemInChoiceItem = false; + boolean itemInChoiceWithoutSigFigs = false; for (CoordinateItem choiceItem : choiceItems) { - if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax)) { + if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, + false)) { submittedItemInChoiceItem = true; break; + } else if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, + true)) { + itemInChoiceWithoutSigFigs = true; } } if (!submittedItemInChoiceItem) { allSubmittedItemsInChoiceItems = false; - break; + // Check if this is just a significant figures mismatch + if (!itemInChoiceWithoutSigFigs) { + allItemsInChoiceWithoutSigFigs = false; + break; + } } } if (allSubmittedItemsInChoiceItems) { feedback = new Content((submittedItems.size() == 1 ? "This is" : "These are") + " correct, but can you find more?"); break; + } else if (allItemsInChoiceWithoutSigFigs) { + feedback = new Content(DEFAULT_VALIDATION_RESPONSE); + feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs", "sig_figs_too_many"))); + break; } } @@ -204,16 +218,24 @@ public final QuestionValidationResponse validateQuestionResponse(final Question boolean allowSubsetMatch = (null != coordinateChoice.isAllowSubsetMatch() && coordinateChoice.isAllowSubsetMatch()); if (allowSubsetMatch && (submittedItems.size() > choiceItems.size())) { boolean allChoiceItemsInSubmittedItems = true; + boolean allItemsInSubmittedWithoutSigFigs = true; for (CoordinateItem choiceItem : choiceItems) { boolean choiceItemInSubmittedItems = false; + boolean itemInSubmittedWithoutSigFigs = false; for (CoordinateItem submittedItem : submittedItems) { - if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax)) { + if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, false)) { choiceItemInSubmittedItems = true; break; + } else if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, true)) { + itemInSubmittedWithoutSigFigs = true; } } if (!choiceItemInSubmittedItems) { allChoiceItemsInSubmittedItems = false; + // Check if this is just a significant figures mismatch + if (!itemInSubmittedWithoutSigFigs) { + allItemsInSubmittedWithoutSigFigs = false; + } break; } } @@ -221,6 +243,10 @@ public final QuestionValidationResponse validateQuestionResponse(final Question responseCorrect = coordinateChoice.isCorrect(); feedback = (Content) coordinateChoice.getExplanation(); break; + } else if (allItemsInSubmittedWithoutSigFigs) { + feedback = new Content(DEFAULT_VALIDATION_RESPONSE); + feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs", "sig_figs_too_many"))); + break; } } } @@ -236,12 +262,19 @@ public final QuestionValidationResponse validateQuestionResponse(final Question if (feedbackIsNullOrEmpty(feedback) && null != coordinateQuestion.getDefaultFeedback()) { feedback = coordinateQuestion.getDefaultFeedback(); } + // If there was no default feedback, check for too few significant figures + if (feedbackIsNullOrEmpty(feedback) && submittedItems.stream().anyMatch(i -> i.getCoordinates().stream() + .anyMatch(c -> ValidationUtils.tooFewSignificantFigures(c, sigFigsMin, log)))) { + feedback = new Content(DEFAULT_VALIDATION_RESPONSE); + feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs", "sig_figs_too_few"))); + } return new QuestionValidationResponse(question.getId(), answer, responseCorrect, feedback, new Date()); } private boolean coordinateItemsMatch(final CoordinateItem submittedItem, final CoordinateItem choiceItem, - final Integer sigFigsMin, final Integer sigFigsMax) { + final Integer sigFigsMin, final Integer sigFigsMax, + final boolean allowTooManySigFigs) { if (submittedItem.getCoordinates().size() != choiceItem.getCoordinates().size()) { return false; @@ -251,6 +284,14 @@ private boolean coordinateItemsMatch(final CoordinateItem submittedItem, final C String submittedValue = submittedItem.getCoordinates().get(dimension); String choiceValue = choiceItem.getCoordinates().get(dimension); + if (allowTooManySigFigs) { + // Check if the submission has more significant figures than the allowed maximum + if (null != sigFigsMax && ValidationUtils.tooManySignificantFigures(submittedValue, sigFigsMax, log)) { + // Check if the submission would match the choice if we ignore the excess significant figures + return ValidationUtils.numericValuesMatch(choiceValue, submittedValue, sigFigsMax, log); + } + } + int sigFigs = ValidationUtils.numberOfSignificantFiguresToValidateWith(submittedValue, sigFigsMin, sigFigsMax, log); if (!ValidationUtils.numericValuesMatch(choiceValue, submittedValue, sigFigs, log) || null != sigFigsMin && ValidationUtils.tooFewSignificantFigures(submittedValue, sigFigsMin, log)