mitodl / mitx-grading-library

The MITx Grading Library, a python grading library for edX
https://edge.edx.org/courses/course-v1:MITx+grading-library+examples/
BSD 3-Clause "New" or "Revised" License
14 stars 9 forks source link

Introducing flag to suppress matrix error messages #158

Closed jolyonb closed 5 years ago

jolyonb commented 5 years ago

This PR introduces a flag to MatrixGrader that completely suppresses all matrix error messages. This is useful when variables are secretly being used as matrices due to their noncommutativity, but matrix error messages would be confusing to students. Includes code, tests, documentation and course example.

ChristopherChudzicki commented 5 years ago

Do you think suppress_shape_messages would be too verbose? Otherwise, this all looks good.

jolyonb commented 5 years ago

That sounds reasonable, and more intuitive, given that it's only suppressing matrix-related messages. I'll go and make the change.

jolyonb commented 5 years ago

Updated the flag name.

ChristopherChudzicki commented 5 years ago

Note that right now suppress_matrix_messages does not suppress all matrix-related errors, just the shape errors. In particular:

from mitxgraders import *
grader = MatrixGrader(
    variables=['A'],
    sample_from={
        'A': RealMatrices()
    },
    max_array_dim=0,
    suppress_messages=True, 
)

# below raises MathArrayError "Cannot raise a matrix to non-integer powers."
result = grader('A', 'A^1.3')

# below raises a DomainError
# (DomainError is not an instance of MathArrayError, but this is matrix-related)
result = grader('A', 'sin(A)')

@jolyon Maybe you should catch MathArrayError instead of ShapeErrors? This would still expose DomainError messages, but those are harder to suppress. Also, see #183 for ideas about an alternative approach.

(Note: the idea in #183 works for customizing DomainError messages, too.)

jolyonb commented 5 years ago

Hmm. I can definitely catch MathArrayErrors and suppress those too. Domain errors ... can also be caught, I think... Let me see what I can do.

jolyonb commented 5 years ago

Ok, I've had a go at fixing things. Let me know what you think. I'm not terribly happy with the outcome, as it's somewhat kludgy, but it's a quick and dirty way to get noncommuting "scalars". I updated the documentation with a warning that taking functions of these "scalars" will be problematic. I'll go and comment on #183 now.