Feature/sig fig#15
Conversation
…lidation and mutual exclusivity checks
…ance handling into dedicated helper functions
peterbjohnson
left a comment
peterbjohnson
left a comment
There was a problem hiding this comment.
Major comment:
- I think we need to sit with Phil and discuss what we mean by sig fig and what the requirements are here. I've made a more detailed note inline in the code.
Minor comment:
- can update all docs and code to the new parameter names (maybe do a sweep) for atol/rtol?
…eros, and enforce string-based responses for accurate significant-figure handling. Updated docs, tests, and logic accordingly.
peterbjohnson
left a comment
peterbjohnson
left a comment
There was a problem hiding this comment.
Comments for your attention but not blocking.
I don't know how this change will work with symbolic answers. E.g. pi? Or x^2?
| ## Parameters | ||
|
|
||
| Both parameters default to `0` (exact match required) and can be used individually or together. | ||
| `atol` and `rtol` both default to `0` (exact match required) and can be used individually or together. `sig_figs` is an alternative comparison mode and cannot be used together with `atol`/`rtol`. |
There was a problem hiding this comment.
Should this use the long form, absolute_tolerance?
| if not isinstance(value, str): | ||
| return None | ||
| stripped = value.strip() | ||
| body = stripped[1:] if stripped[:1] in ("+", "-") else stripped |
There was a problem hiding this comment.
Will we also accept plus/minus (pm)? We have some accommodation for that concept. But perhaps it's at the preview stage which splits it into a list of multiple entries?
| if sep and not exponent.lstrip("+-").isdigit(): | ||
| return None | ||
|
|
||
| has_decimal = "." in mantissa |
|
|
||
| feedback = "" | ||
| if numeric_correct and not precision_correct: | ||
| feedback = f"Please give your answer to {sig_figs} significant figures." |
There was a problem hiding this comment.
I think Karl started collecting all feedback strings in one place, so that they can be easily maintained. Could you find that and put this string there and call it?
|
Thanks for the feedback. Just to note that this is IsSimilar and not compareExpressions. I need to migrate the functionality across. Can you confirm whether we want to support sig fig in isSimilar or only in compareExpressions? |
|
I think it's best to keep isSimilar as assume as possible. So porting to cE is best. |
2 participants
Requires #14
Problem
IsSimilar should also support significant figures and tolerances.
Changes