Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

is_different behavior differs with numpy vs without numpy #930

Open
thequilo opened this issue Aug 26, 2024 · 6 comments
Open

is_different behavior differs with numpy vs without numpy #930

thequilo opened this issue Aug 26, 2024 · 6 comments

Comments

@thequilo
Copy link
Collaborator

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.

@IDSIA IDSIA deleted a comment Aug 26, 2024
@n-gao
Copy link
Contributor

n-gao commented Oct 14, 2024

@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
Copy link
Collaborator Author

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

@n-gao
Copy link
Contributor

n-gao commented Nov 7, 2024

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

@thequilo
Copy link
Collaborator Author

thequilo commented Nov 9, 2024

@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
Copy link
Contributor

n-gao commented Nov 18, 2024

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

@thequilo
Copy link
Collaborator Author

Done!

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 a pull request may close this issue.

2 participants