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

Warning about double-registration #171

Open
nstarman opened this issue Jun 27, 2024 · 8 comments
Open

Warning about double-registration #171

nstarman opened this issue Jun 27, 2024 · 8 comments

Comments

@nstarman
Copy link
Contributor

In GalacticDynamics/coordinax#128 I am trying to build with plum v2.4.3, but I am seeing many MethodRedefinitionWarning warnings about my constructor methods, but the earlier definition it compares against is the same function!

e.g.

ERROR tests/test_base.py - plum.resolver.MethodRedefinitionWarning: Method(function_name='constructor', signature=Signature(type[coordinax._base.AbstractVector], collections.abc.Mapping[str, unx, return_type=<class 'coordinax._base.AbstractVector'>, impl=<function AbstractVector.constructor at 0x7fc60876f2e0>) overwrites the earlier definition Method(function_name='constructor', signature=Signature(type[coordinax._base.AbstractVector], collections.abc.Mapping[str, unx, return_type=<class 'coordinax._base.AbstractVector'>, impl=<function AbstractVector.constructor at 0x7fc60876f2e0>).

where both functions are 0x7fc60876f2e0.

@nstarman
Copy link
Contributor Author

nstarman commented Jun 27, 2024

@patrick-kidger, I managed to isolate the problem somewhat and Equinox is involved.
It doesn't happen when I use a dataclasses.dataclass instead.

Here'a MWE that raises a warning

from collections.abc import Mapping
from dataclasses import dataclass

import equinox as eqx
from plum import dispatch

class Class(eqx.Module):
    a: float
    b: float

    @classmethod
    @dispatch
    def constructor(cls: "type[Class]", obj: Mapping[str, float], /) -> "Class":
        return cls(**obj)

Emits MethodRedefinitionWarning: `Method(function_name='constructor', signature=Signature(type[__main__.Class], collections.abc.Mapping[str, float]), return_type=<class '__main__.Class'>, impl=<function Class.constructor at 0x177cdade0>)` overwrites the earlier definition `Method(function_name='constructor', signature=Signature(type[__main__.Class], collections.abc.Mapping[str, float]), return_type=<class '__main__.Class'>, impl=<function Class.constructor at 0x177cd80e0>)`.

@nstarman
Copy link
Contributor Author

nstarman commented Jun 28, 2024

Sorry @patrick-kidger, that may hav been a spurious finding. When I updated equinox this wasn't sourced by equinox any more.

@wesselb
Copy link
Member

wesselb commented Jun 28, 2024

@nstarman, hmmm, I seem to be unable to reproduce the issue in a fresh environment with Python 3.10:

Python 3.10.13 (main, Jan 16 2024, 14:46:45) [Clang 14.0.0 (clang-1400.0.29.202)]
Type 'copyright', 'credits' or 'license' for more information
IPython 8.26.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from collections.abc import Mapping
   ...: from dataclasses import dataclass
   ...:
   ...: import equinox as eqx
   ...: from plum import dispatch
   ...:
   ...: class Class(eqx.Module):
   ...:     a: float
   ...:     b: float
   ...:
   ...:     @classmethod
   ...:     @dispatch
   ...:     def constructor(cls: "type[Class]", obj: Mapping[str, float], /) -> "Class":
   ...:         return cls(**obj)
   ...:

In [2]: Class.constructor._f.methods
Out[2]:
List of 1 method(s):
    [0] constructor(cls: type, obj: collections.abc.Mapping) -> __main__.Class
        <function Class.constructor at 0x10a530af0> @ /<ipython-input-1-9a739e5fe7aa>:11

Could it possibly be the case that you accidentally ran the snippet twice in the same session? That would produce a redefinition error:

In [1]: from collections.abc import Mapping
   ...: from dataclasses import dataclass
   ...:
   ...: import equinox as eqx
   ...: from plum import dispatch
   ...:
   ...: class Class(eqx.Module):
   ...:     a: float
   ...:     b: float
   ...:
   ...:     @classmethod
   ...:     @dispatch
   ...:     def constructor(cls: "type[Class]", obj: Mapping[str, float], /) -> "Class":
   ...:         return cls(**obj)
   ...:

In [2]: Class.constructor._f.methods
Out[2]:
List of 1 method(s):
    [0] constructor(cls: type, obj: collections.abc.Mapping) -> __main__.Class
        <function Class.constructor at 0x10a530af0> @ /<ipython-input-1-9a739e5fe7aa>:11

In [3]: from collections.abc import Mapping
   ...: from dataclasses import dataclass
   ...:
   ...: import equinox as eqx
   ...: from plum import dispatch
   ...:
   ...: class Class(eqx.Module):
   ...:     a: float
   ...:     b: float
   ...:
   ...:     @classmethod
   ...:     @dispatch
   ...:     def constructor(cls: "type[Class]", obj: Mapping[str, float], /) -> "Class":
   ...:         return cls(**obj)
   ...:
/Users/wessel/Desktop/venv/lib/python3.10/site-packages/plum/resolver.py:269: MethodRedefinitionWarning: `Method(function_name='constructor', signature=Signature(type[__main__.Class], collections.abc.Mapping[str, float]), return_type=<class '__main__.Class'>, impl=<function Class.constructor at 0x103b94280>)` overwrites the earlier definition `Method(function_name='constructor', signature=Signature(type[__main__.Class], collections.abc.Mapping[str, float]), return_type=<class '__main__.Class'>, impl=<function Class.constructor at 0x10a530af0>)`.
  warnings.warn(

@wesselb
Copy link
Member

wesselb commented Jun 28, 2024

Oops, I completely missed your last message, @nstarman! Glad that the issue seems to have gone away. :) Is this ready to be closed?

@nstarman
Copy link
Contributor Author

I'm still getting the issue everywhere in GalacticDynamics/coordinax#128. It's just not (maybe?) caused by Equinox. I'm suspicious of jaxtyping.install_import_hook ATM, but have no proof.

@wesselb
Copy link
Member

wesselb commented Jun 29, 2024

Hmm, let's leave it open for now then to see if anything else comes up. I'll try to release the warn_redefinition keyword soon, which should be an easy change.

@nstarman
Copy link
Contributor Author

Great. I'll probably use that in coordinax so I can unpin plum. Then I can continue trying to find a MWE.

@wesselb
Copy link
Member

wesselb commented Jul 6, 2024

@nstarman v2.5.1 makes redefinition warnings opt-in by setting warn_redefinition=True in Dispatcher, Function, or Resolver.

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

No branches or pull requests

2 participants