IDSIA / sacred

Sacred is a tool to help you configure, organize, log and reproduce experiments developed at IDSIA.
MIT License
4.26k stars 383 forks source link

is_different behavior differs with numpy vs without numpy #930

Open thequilo opened 2 months ago

thequilo commented 2 months ago

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.

n-gao commented 1 month ago

@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,)
thequilo commented 1 month ago

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

n-gao commented 2 weeks ago

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

thequilo commented 1 week ago

@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.

n-gao commented 4 days ago

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