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

[BUG] Can't initialize qml.GateFabric with wires as kwargs. #5521

Closed
1 task done
dwierichs opened this issue Apr 16, 2024 · 3 comments
Closed
1 task done

[BUG] Can't initialize qml.GateFabric with wires as kwargs. #5521

dwierichs opened this issue Apr 16, 2024 · 3 comments
Labels
bug 🐛 Something isn't working wontfix 🙈 This will not be worked on

Comments

@dwierichs
Copy link
Contributor

dwierichs commented Apr 16, 2024

Expected behavior

Like any PennyLane operation, qml.GateFabric should be able to initialize with wires given as kwarg, even when not giving init_state as kwarg.

Actual behavior

The GateFabric errors out, unless init_state is provided as kwarg as well.

edit
AllSinglesDoubles has the same problem, with hf_state being the offending positional argument.
BasisRotation has the same problem, with unitary_matrix being the offending positional argument.

Additional information

This error is caused by GateFabric allowing wires as a positional arg but not as the last positional arg, which is a requirement for PennyLane operations.
We should either change the order of the positional args or make init_state a kwarg.

Source code

qml.GateFabric(np.ones((3, 1, 2)), [0, 1, 1, 0], wires=[2, 3, 0, 1])

weights = np.ones(3)
singles = [[0, 2], [1, 3]]
doubles = [[0, 1, 2, 3]]
hf_state = [1, 1, 0, 0]

qml.AllSinglesDoubles(weights, hf_state, wires=[0, 1, 2, 3], singles=singles, double=doubles)

Tracebacks

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[5], line 1
----> 1 qml.GateFabric(np.ones((3, 1, 2)), [0, 1, 1, 0], wires=[2, 3, 0, 1])

File ~/repos/pennylane/pennylane/capture/meta_type.py:151, in PLXPRMeta.__call__(cls, *args, **kwargs)
    145 def __call__(cls, *args, **kwargs):
    146     # this method is called everytime we want to create an instance of the class.
    147     # default behavior uses __new__ then __init__
    148     # when tracing is enabled, we want to
    150     if not plxpr_enabled():
--> 151         return type.__call__(cls, *args, **kwargs)
    152     # use bind to construct the class if we want class construction to add it to the jaxpr
    153     return cls._primitive_bind_call(*args, **kwargs)

TypeError: GateFabric.__init__() got multiple values for argument 'wires'

System information

pl dev

Existing GitHub issues

  • I have searched existing GitHub issues to make sure the issue does not already exist.
@dwierichs dwierichs added the bug 🐛 Something isn't working label Apr 16, 2024
@PietropaoloFrisoni
Copy link
Contributor

PietropaoloFrisoni commented May 2, 2024

Hi @dwierichs, just one point. If we make init_state a kwarg, this would fulfill the requirement to have wires as the last positional argument, but the example above:

qml.GateFabric(np.ones((3, 1, 2)), [0, 1, 1, 0], wires=[2, 3, 0, 1])

still would not work, so I think the only option would be to change the argument order. If we do that, then the above line of code works fine, but for:

qml.AllSinglesDoubles(weights, hf_state, wires=[0, 1, 2, 3], singles=singles, doubles=doubles)

we get a different error:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
(...)
AttributeError: 'int' object has no attribute 'dtype'

@dwierichs
Copy link
Contributor Author

Hi @PietropaoloFrisoni ,thanks for looking into this!

but the example above: [...] still would not work,

I agree, but I think that if we made init_state a kwarg, users would not try the above (because it is clear that Python would not map the init_state correctly to the kwarg), whereas the current syntax/documentation suggests that it is a valid input format.

we get a different error:

Yeah, the input validation is not optimal, it assumes ndarray(int) and does not work with list[int]. But that's independent and also a problem with the current code.

@albi3ro
Copy link
Contributor

albi3ro commented May 9, 2024

While this does break pennylane convention, it doesn't actually break any code. And though a different call signature would be better, it's not worth the problems and costs that accompany a breaking change of this sort.

After discussion with relevant stakeholders, we are marking this as a "wont-fix".

@albi3ro albi3ro closed this as completed May 9, 2024
@albi3ro albi3ro added the wontfix 🙈 This will not be worked on label May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working wontfix 🙈 This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants