minitorch / Module-1

Module 1 - Autodifferentiation
20 stars 204 forks source link

Alternate solution for Task 1.2 does not pass tests #11

Open tnwei opened 2 years ago

tnwei commented 2 years ago

For Task 1.2, I believe the intended code for scalar.Scalar.__sub__is return Add.apply(self, -b), which passes the tests. Interestingly, if return Add.apply(self, Neg.apply(b)) is used, the tests that involve subtracting constants fail. Excerpt of the failed test cases below:

===============================short test summary info==============================
FAILED tests/test_scalar.py::test_one_args[fn4] - AssertionError: Expected return typ <class 'float'> got <class 'int'>: -200
FAILED tests/test_scalar.py::test_one_args[fn5] - AssertionError: Expected return typ <class 'float'> got <class 'int'>: -200
FAILED tests/test_scalar.py::test_one_args[fn13] - AssertionError: Expected return typ <class 'float'> got <class 'int'>: -5     

I've slightly modified the error message in autodiff.FunctionBase to print the offending value in question, and they are all ints. The root cause is due to scalar.ScalarFunction.data_type being hard-coded as float. I assume data_type is fixed for a reason, and there probably isn't an easy fix to allow the alternate solution to pass the tests. Just thought y'all should know.

srush commented 2 years ago

Thanks! Someone else mentioned this as well. Needs to be fixed.

ingambe commented 2 years ago

Indeed, this issue occurs when all the operations do not return a float. I manage to pass the test with Add.apply(self, Neg.apply(b)) by defining the negation operation as: return -1.0 * x