qutip / qutip-jax

JAX backend for QuTiP
BSD 3-Clause "New" or "Revised" License
15 stars 7 forks source link

Number of parameters to inverse specialisation seems to be incorrect #13

Closed quantshah closed 1 year ago

quantshah commented 1 year ago

On current master, if I try to import qutip_jax, I get an error. The issue is that inverse specialisation seems to have been defined incorrectly. The following test tells you that the number of parameters in the specialisation specification is incorrect and it requires a (DataType, Callable). Current the specialisation is added as:

qutip.data.inv.add_specialisations(
    [
        (JaxArray, JaxArray, inv_jaxarray),
    ]
)

but it seems the following is the correct way:

qutip.data.inv.add_specialisations(
    [
        (JaxArray, inv_jaxarray),
    ]
)

The tests pass with the 2nd specification. Either the core inversion specialisation has been setup that way and we need to use the 2nd definition with only (DataType, Callable) for inversion, or something is wrong with the core inversion definition in the dispatcher.

@Ericgig @jakelishman

Full error message:

pytest test_unary.py -k 'TestInv'
ImportError while loading conftest '/Users/shahnawaz/Dropbox/dev/qutip-jax/tests/conftest.py'.
conftest.py:2: in <module>
    import qutip_jax
../src/qutip_jax/__init__.py:27: in <module>
    from .unary import *
../src/qutip_jax/unary.py:120: in <module>
    qutip.data.inv.add_specialisations(
qutip/core/data/dispatch.pyx:471: in qutip.core.data.dispatch.Dispatcher.add_specialisations
    ???
E   ValueError: specialisation (<class 'qutip_jax.jaxarray.JaxArray'>, <class 'qutip_jax.jaxarray.JaxArray'>, <function inv_jaxarray at 0x138bc5310>) has wrong number of parameters: needed types for ('data',) and a callable
jakelishman commented 1 year ago

Looks like it was set up incorrectly (as you've found), but has been fixed by qutip/qutip#2006 to me - I just had a glance through the git blame.

quantshah commented 1 year ago

Ah great. I will just pull the updated dev. Thanks @jakelishman. Btw the whole data layer and the testing framework for it is just awesome, great job 🤌

jakelishman commented 1 year ago

haha, thanks! All I can think about it with it is that I never had time to get a bunch more of the dynamic switching / dispatch-control functionality that I wanted written :(. But I'm happy with what we did get in!