Skip to content

Bug/fix inappropriate symbol and ! for factorial#255

Merged
m-messer merged 15 commits into
mainfrom
bug/fix-inappropriate-symbol-and-!-for-factorial
May 19, 2026
Merged

Bug/fix inappropriate symbol and ! for factorial#255
m-messer merged 15 commits into
mainfrom
bug/fix-inappropriate-symbol-and-!-for-factorial

Conversation

@m-messer
Copy link
Copy Markdown
Member

@m-messer m-messer commented May 13, 2026

Requires #247 to be merged first before merging this PR.

Problem

Two bugs affecting infinity and factorial handling in symbolic expression comparison:

  1. The Unicode infinity symbol (∞) was not recognised as valid input, causing parse errors when students used it.
  2. Comparing infinite expressions (e.g. oo = oo) would subtract the two sides before checking, but oo - oo evaluates to nan in SymPy rather than 0, causing incorrect results. A KeyError/TypeError could also be thrown during the is_infinite check.
  3. The triple factorial notation !!! was silently mishandled — it parses as repeated ! (factorial of factorial of factorial) rather than the mathematical triple factorial, with no warning to the student.

Fix

  • Added ∞ as an alias for oo in expression_utilities.py so the Unicode symbol is accepted as input.
  • Added do_comparison_infinite in context/symbolic.py that compares infinite expressions directly (e.g. lhs == rhs) instead of via subtraction, and wrapped the is_infinite check in a try/except to guard against edge-case exceptions.
  • Added a NOTATION_WARNING_TRIPLE_FACTORIAL feedback message and wired it up in evaluation.py to warn students that !!! is not supported.

Other changes

  • Upgraded to Python 3.12, SymPy 1.14, and mpmath 1.4.1; updated CI workflows accordingly.
  • Fixed the Dockerfile: switched base image package manager, added find to installed requirements, set PYTHONPATH, and updated the CMD entrypoint format.

@m-messer
Copy link
Copy Markdown
Member Author

Database testing can be found here: https://github.com/lambda-feedback/compareExpressions/actions/runs/25786405204/job/75740792212

7 errors out of 1000

  1. _Grade Mismatch - eval func True, DB False -- Answer 2(cos(pi/2)+isin(pi/2)), response 2( cos(π/2)+i sin(π/2))
  • Added support for unicode greek letters in a previous fix
  1. E^{x log x} ( log x + 1) could not be parsed as a valid mathematical expression. Ensure that correct codes for input symbols are used, correct notation is used, that the expression is unambiguous and that all parentheses are closed.
    • Answer written in LaTeX not Sympy
  2. 2x \ sin x + x^{2} \ cos x could not be parsed as a valid mathematical expression. Ensure that correct codes for input symbols are used, correct notation is used, that the expression is unambiguous and that all parentheses are closed.
  • Same as 2.
  1. x E^{x}(2+x) could not be parsed as a valid mathematical expression. Ensure that correct codes for input symbols are used, correct notation is used, that the expression is unambiguous and that all parentheses are closed.
  • Same as 2.
  1. asec**2(ax) could not be parsed as a valid mathematical expression. Ensure that correct codes for input symbols are used, correct notation is used, that the expression is unambiguous and that all parentheses are closed.
  • Invalid form for asec^2(ax), should be asec(ax)**2
  1. _Grade Mismatch - eval func True, DB False -- Answer -5 muA, response -5 µA
  • Added support for unicode greek letters in a previous fix
  1. (x cosx - sinx) / (x^{2}) could not be parsed as a valid mathematical expression. Ensure that correct codes for input symbols are used, correct notation is used, that the expression is unambiguous and that all parentheses are closed.
  • Same as 2.

Copy link
Copy Markdown
Member

@peterbjohnson peterbjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All ok with me and I've added an inline comment from Phil on the comparison of infinities. We'll do it and see how it goes.

Comment thread app/context/symbolic.py
def do_comparison_infinite(comparison_symbol, lhs_expr, rhs_expr):
# When either side is infinite, subtracting (e.g. oo - oo) yields nan, making
# the subtraction-based do_comparison unreliable. Compare the sides directly instead.
direct_comparisons = {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment from Phil Ramsden after I asked him about this:

"I'm not speaking here as a mathematician exactly, but as someone who's written this kind of code for METRIC back in the day! The rationale for comparing the difference to zero is that zero is unambiguously "simpler" than almost any expression equivalent to it, and is therefore a likely end point for any chain of simplification. By contrast, it's possible to imagine (and not all that hard to concoct) examples of expressions that are algebraically equivalent but don't simplify to the same thing.

However, there are, potentially, several pitfalls with "compare difference to zero". Infinity is one of them, as there's no well-defined difference operation for infinities. A slightly different class of issue arises where the standard difference operation on equivalent objects exists all right, but yields something other than zero, such as the zero vector or (perhaps, depending on how difference is set up) the empty set. So "compare difference to zero" is not a panacea, and should only really be used when comparing symbolic expressions that represent scalars of some sort."

@m-messer m-messer merged commit ff7ca73 into main May 19, 2026
10 checks passed
@m-messer m-messer deleted the bug/fix-inappropriate-symbol-and-!-for-factorial branch May 19, 2026 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants