sacred icon indicating copy to clipboard operation
sacred copied to clipboard

is_different behavior differs with numpy vs without numpy

Open thequilo opened this issue 1 year ago • 6 comments

In #928 the issue came up that the behavior of is_different in custom_containers differs if numpy is installed vs when it is not installed. Here is an example:

>>> import sacred

>>> sacred.config.custom_containers.is_different([1, [1,2]], [1, [1,2]])
True

>>> sacred.optional.has_numpy = False

>>> sacred.config.custom_containers.is_different([1, [1,2]], [1, [1,2]])
False

This looks like undesired behavior, but I have to investigate further what impact this issue has before I fix it.

thequilo avatar Aug 26 '24 08:08 thequilo

@thequilo I just noticed that #928 did introduce some unintended changes. With numpy haven released fixes to restore the old array_equal functionality, could we revert these changes?

This one breaks now:

from sacred import Experiment

ex = Experiment()


@ex.config
def config():
    a = dict(b=[])


@ex.automain
def main(a):
    return a

python test.py with a.b='["hello", "world"]':

Exception originated from within Sacred.
Traceback (most recent calls):
  File "/ceph/ssd/staff/gaoni/repos/sparse_wf/.venv/lib/python3.11/site-packages/sacred/config/custom_containers.py", line 312, in is_different
    result = old_value != new_value
             ^^^^^^^^^^^^^^^^^^^^^^
ValueError: operands could not be broadcast together with shapes (0,) (2,)

n-gao avatar Oct 14 '24 17:10 n-gao

Yeah, I forgot the if old_value.shape != new_value.shape: return True part in #928

thequilo avatar Oct 15 '24 14:10 thequilo

@thequilo could we please make a new release? The latest release (before #933) breaks a lot of configurations.

n-gao avatar Nov 07 '24 16:11 n-gao

@n-gao sorry for the late response, I'm currently out of the office. Does the current master work? If so, I can do a quick bugfix release.

thequilo avatar Nov 09 '24 08:11 thequilo

@thequilo Sorry for the late reply. The master branch fixes my issues :)

n-gao avatar Nov 18 '24 15:11 n-gao

Done!

thequilo avatar Nov 26 '24 07:11 thequilo