diff --git a/evaluation_function/compare_MIDI.py b/evaluation_function/compare_MIDI.py index 8d72cc4..41c65fe 100644 --- a/evaluation_function/compare_MIDI.py +++ b/evaluation_function/compare_MIDI.py @@ -40,6 +40,28 @@ # Default threshold: notes starting within 50ms are grouped as one chord. DEFAULT_CHORD_ONSET_WINDOW = 0.05 +# Feedback for the two degenerate cases, where there is nothing to compare. +# These are named rather than written inline so that tests can assert which +# case was hit without depending on the wording, which is free to change. +NO_REFERENCE_NOTES_MESSAGE = "\n".join([ + "Practice Summary", + "This question has no reference notes to compare your performance " + "against, so it could not be evaluated. Please let your teacher know.", +]) + +NO_RESPONSE_NOTES_MESSAGE = "\n".join([ + "Practice Summary", + "No notes were detected in your submission, so there was nothing " + "to compare against the reference.", + "", + "What to check", + "If you submitted a recording, check that it is not silent and that " + "your instrument can be heard clearly. If you submitted MIDI, check " + "that it contains notes.", + "", + "Have another go when you are ready.", +]) + # template and helper functions for chords # ------------------------------------------------------------------------------ # Chord template dictionary. @@ -285,18 +307,25 @@ def build_cost_matrix(response_events, ref_events, gap_penalty=DEFAULT_GAP_PENAL # Build simple per-event arrays: is this event a chord, and (if it is a # single note) what is its pitch. - res_is_chord = np.array([event["event_type"] == "chord" for event in response_events]) - ref_is_chord = np.array([event["event_type"] == "chord" for event in ref_events]) + # The dtypes are given explicitly so that an empty event list still + # produces bool/int arrays; numpy would otherwise default them to float + # and the boolean masks below would fail. + res_is_chord = np.array( + [event["event_type"] == "chord" for event in response_events], dtype=bool + ) + ref_is_chord = np.array( + [event["event_type"] == "chord" for event in ref_events], dtype=bool + ) # For note events, extract the pitch; for chords, use 0 as a placeholder. res_pitch = np.array([ event["notes"][0]["pitch"] if event["event_type"] == "note" else 0 for event in response_events - ]) + ], dtype=int) ref_pitch = np.array([ event["notes"][0]["pitch"] if event["event_type"] == "note" else 0 for event in ref_events - ]) + ], dtype=int) # Note-vs-note cost: vectorised absolute pitch difference for every pair. # Shape (N, 1) - shape (1, M) broadcasts to (N, M) @@ -351,10 +380,12 @@ def event_alignment_ED(response_events, ref_events, gap_penalty=DEFAULT_GAP_PENA D: accumulated cost matrix, shape (N+1, M+1) """ # if a raw note dict with "pitch"/"start"/"duration" but no "event_type" is - # passed in, group them into events first. - if "event_type" not in response_events[0]: + # passed in, group them into events first. + # An empty list has nothing to inspect, and nothing to group either, so + # skip the check rather than indexing into it. + if response_events and "event_type" not in response_events[0]: response_events = group_notes_into_events(response_events) - if "event_type" not in ref_events[0]: + if ref_events and "event_type" not in ref_events[0]: ref_events = group_notes_into_events(ref_events) # the rows of D correspond to response events @@ -1087,6 +1118,16 @@ def polished_feedback_message(event_details, response_events, ref_events, stats, Returns: feedback_message (str) """ + # Degenerate submissions, handled before the tiered messages below. + # The usual wording would be actively misleading here: telling a student + # who submitted nothing that they "missed" every note, or praising a + # perfect match against a reference that contains no notes at all. + if len(ref_events) == 0: + return NO_REFERENCE_NOTES_MESSAGE + + if len(response_events) == 0: + return NO_RESPONSE_NOTES_MESSAGE + note_events = [n for n in event_details if n["event_type"] == "note"] chord_events = [ch for ch in event_details if ch["event_type"] == "chord"] @@ -1434,8 +1475,13 @@ def compare_performance_ED(responseMIDI, refMIDI, ) # Step 6: Overall pass/fail judgement + # A submission with no notes on either side cannot be correct, even though + # the counts below are all trivially satisfied when there is nothing to + # compare. is_correct = ( - stats["total_notes_missing"] == 0 + len(response_events) > 0 + and len(ref_events) > 0 + and stats["total_notes_missing"] == 0 and stats["total_notes_extra"] == 0 and stats["total_chords_missing"] == 0 and stats["total_chords_extra"] == 0 diff --git a/evaluation_function/evaluation_test.py b/evaluation_function/evaluation_test.py index 9aca78e..9cb3468 100755 --- a/evaluation_function/evaluation_test.py +++ b/evaluation_function/evaluation_test.py @@ -19,6 +19,7 @@ 8. Tests for evaluation_function (Lambda Feedback integration) 9. Tests for parameter overrides 10. Bulk tests using longer MIDI sequences +11. Tests for submissions that contain no notes """ @@ -37,6 +38,8 @@ event_level_feedback, compute_stats, compare_performance_ED, + NO_REFERENCE_NOTES_MESSAGE, + NO_RESPONSE_NOTES_MESSAGE, DEFAULT_GAP_PENALTY, TIMING_RELATIVE_THRESHOLD, DURATION_RELATIVE_THRESHOLD, @@ -705,4 +708,102 @@ def test_realistic_scenario(case): ] assert len(matching_notes) == 1 flagged_note = matching_notes[0] - assert flagged_note["timing_correct"] is False \ No newline at end of file + assert flagged_note["timing_correct"] is False + +# 11. Tests for submissions that contain no notes +# ------------------------------------------------------------------------------ +# An empty note list is reachable in production: a student submits nothing, +# uploads a silent or failed recording, or plays so quietly that transcription +# returns no notes at all. These cases used to raise IndexError, which reaches +# the student as a 500 rather than a feedback message. +# +# Short-but-not-empty submissions already worked, and are covered here so they +# stay working. + +EMPTY_MIDI = {"notes": []} + + +def short_melody(note_count): + """The first note_count notes of a simple four-note melody.""" + pitches = [60, 62, 64, 65][:note_count] + starts = [0.0, 0.5, 1.0, 1.5][:note_count] + return make_midi(pitches, starts, [0.4] * note_count) + + +FOUR_NOTE_REFERENCE = short_melody(4) + + +class TestEmptyResponse(unittest.TestCase): + + def test_does_not_raise(self): + compare_performance_ED(EMPTY_MIDI, FOUR_NOTE_REFERENCE) + + def test_is_not_correct(self): + result = compare_performance_ED(EMPTY_MIDI, FOUR_NOTE_REFERENCE) + assert result.is_correct is False + + def test_every_reference_note_counted_as_missing(self): + result = compare_performance_ED(EMPTY_MIDI, FOUR_NOTE_REFERENCE) + assert result.stats["total_notes_missing"] == 4 + assert result.stats["total_notes_extra"] == 0 + + def test_feedback_says_no_notes_were_detected(self): + # "You missed four notes" is technically true but unhelpful when the + # student submitted nothing at all. Compare against the constant the + # code returns, so that rewording the message does not break this. + result = compare_performance_ED(EMPTY_MIDI, FOUR_NOTE_REFERENCE) + assert result.feedback_message == NO_RESPONSE_NOTES_MESSAGE + + def test_through_the_platform_entry_point(self): + result = evaluation_function(EMPTY_MIDI, FOUR_NOTE_REFERENCE, {}) + assert result["is_correct"] is False + assert result["feedback"] == NO_RESPONSE_NOTES_MESSAGE + + +class TestEmptyReference(unittest.TestCase): + """An empty reference is a misconfigured question, not a student error.""" + + def test_does_not_raise(self): + compare_performance_ED(FOUR_NOTE_REFERENCE, EMPTY_MIDI) + + def test_is_not_correct(self): + result = compare_performance_ED(FOUR_NOTE_REFERENCE, EMPTY_MIDI) + assert result.is_correct is False + + def test_feedback_points_at_the_question_not_the_student(self): + result = compare_performance_ED(FOUR_NOTE_REFERENCE, EMPTY_MIDI) + assert result.feedback_message == NO_REFERENCE_NOTES_MESSAGE + + +class TestBothEmpty(unittest.TestCase): + + def test_does_not_raise(self): + compare_performance_ED(EMPTY_MIDI, EMPTY_MIDI) + + def test_is_not_correct(self): + # A submission with nothing to compare cannot be correct, even though + # an empty response trivially "matches" an empty reference. + result = compare_performance_ED(EMPTY_MIDI, EMPTY_MIDI) + assert result.is_correct is False + + def test_reports_the_missing_reference_rather_than_the_empty_response(self): + # With nothing on either side, the misconfigured question is the more + # useful thing to report, so that branch must win. + result = compare_performance_ED(EMPTY_MIDI, EMPTY_MIDI) + assert result.feedback_message == NO_REFERENCE_NOTES_MESSAGE + + +class TestShortSubmissions(unittest.TestCase): + """One and two note submissions already worked; keep them working.""" + + def test_one_note_against_four_note_reference(self): + result = compare_performance_ED(short_melody(1), FOUR_NOTE_REFERENCE) + assert result.stats["total_notes_missing"] == 3 + + def test_two_notes_against_four_note_reference(self): + result = compare_performance_ED(short_melody(2), FOUR_NOTE_REFERENCE) + assert result.stats["total_notes_missing"] == 2 + + def test_single_note_matching_single_note_reference_is_correct(self): + one_note = short_melody(1) + assert compare_performance_ED(one_note, one_note).is_correct is True