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

Wrong parameters assignment in the tutorial 11_quantum_convolutional_neural_networks #678

Closed
derwind opened this issue Aug 26, 2023 · 5 comments · Fixed by #728
Closed

Comments

@derwind
Copy link

derwind commented Aug 26, 2023

Environment

  • Qiskit Machine Learning version: 0.6.1
  • Python version: 3.10.9
  • Operating system: Linux (Ubuntu 20.04)

What is happening?

I suspect that wrong parameters assignment occurs in the NeuralNetworkClassifier of 11_quantum_convolutional_neural_networks.html.

How can we reproduce the issue?

It is difficult to explain in words, so I created a notebook: 11_quantum_convolutional_neural_networks_issue.ipynb

So you can reproduce the issue by executing the attached notebook.


In short, train_images data are assigned to the conv layer whose name is "c2" and the parameters from 11_qcnn_initial_point.json are assigned to ParameterVectorElement(c2[8]), ParameterVectorElement(c2[9]), ...

The contents up to cell No.14 are the same as in the tutorial. However, only cell No.13 has maxiter=0 to finish immediately. My explanation will start from the cell No.15.

What should happen?

I think train_images data should be assigned to x[0], x[1], ... and the parameters from 11_qcnn_initial_point.json should be assigned to c1[0], c1[1], ...

Note that c of c1 in the tutorial is actually not a latin characterc (LATIN SMALL LETTER C; U+0063) but a cyrillic character с (CYRILLIC SMALL LETTER ES; U+0441). After c2, c of c2, c3 are LATIN SMALL LETTER C.

Any suggestions?

No response

@woodsp-ibm
Copy link
Member

Please look at the linked PR above #728. That addressed a different issue to start with but I needed to address the notebook you refer to in this issue where I saw the problem with "c1" and changed it. I believe the PR addresses this issue so I set it to close it.

@derwind
Copy link
Author

derwind commented Dec 15, 2023

@woodsp-ibm Thanks for your reply. I have not patched and run the codes, however I have looked at the implementation of _reparameterize_circuit, and I feel that the type of the new_parameters passed to assign_parameters seems to be Mapping[Parameter, ParameterVectorElement]. I believe that Mapping[Parameter, ParameterValueType] is expected in the original QuantumCircuit.assign_parameters's specification.

@woodsp-ibm
Copy link
Member

woodsp-ibm commented Dec 15, 2023

A ParameterVectorElement is a (sub-class of) Parameter so its a Mapping of Parameter to Parameter that is passed to replace the original named parameters with new ones so the overall circuit has the desired order of the parameters. So rather than setting a value to the Parameter its replacing the Parameter entirely by a different one. In the assign_parameters method you will see you can pass a Mapping[Parameter, ParameterExpression | float]. So the type of the second parameter determines if you are setting a value in it or replacing it. Now I said Parameter to Parameter was the type passed, a Parameter is a (sub-class of) ParmeterExpression, so it matches to what assign_parameters needs. You can see an example of this here https://docs.quantum.ibm.com/api/qiskit/qiskit.circuit.QuantumCircuit#examples-2

@derwind
Copy link
Author

derwind commented Dec 15, 2023

OK... I checked Qiskit's source codes and recognized that Parameter is a subclass of ParameterExpression. I now feel your PR is appropriate.

I understand that, as the alphabetical order,

>>> "inputs" < "weights"
True

so, ParameterVector("inputs", ...) precede ParameterVector("weights", ...) in parameters of QuantumCircuit.

I also checked by writing snippets below with being aware "in" < "w" < "x":

>>> from qiskit import QuantumCircuit
>>> from qiskit.circuit import ParameterVector
>>> x = ParameterVector("x", 1)
>>> w = ParameterVector("w", 1)
>>> qc = QuantumCircuit(1)
>>> qc.ry(x[0], 0)
<qiskit.circuit.instructionset.InstructionSet object at 0x7fa3fc8e5bd0>
>>> qc.ry(w[0], 0)
<qiskit.circuit.instructionset.InstructionSet object at 0x7fa3fc8e5c90>
>>> qc.draw("text")
   ┌──────────┐┌──────────┐
q: ┤ Ry(x[0]) ├┤ Ry(w[0]) ├
   └──────────┘└──────────┘
>>> qc.parameters
ParameterView([ParameterVectorElement(w[0]), ParameterVectorElement(x[0])])
>>> new_x = ParameterVector("in", 1)
>>> qc = qc.assign_parameters({x[0]: new_x[0]})
>>> qc.parameters
ParameterView([ParameterVectorElement(in[0]), ParameterVectorElement(w[0])])
>>> qc.draw("text")
   ┌───────────┐┌──────────┐
q: ┤ Ry(in[0]) ├┤ Ry(w[0]) ├
   └───────────┘└──────────┘

@woodsp-ibm
Copy link
Member

Right what's important in the end, is that the data is bound by the primitive (Estimator or Sampler) where the the list of values passed is bound in the order that qc.parameters returns. As the data in the QNNs is ordered inputs then outputs in the data values array passed the parameters need to be the same order. With the standard library feature map circuit using "x" and circuit library anstazes using greek theta this was normally the case but that cannot be guaranteed for any circuit. Hence this rewriting ensures this is the case, that what the user says are input and weight params in the sequences passed, the data truly gets bound in that order no matter the original naming.

@mergify mergify bot closed this as completed in #728 Jan 8, 2024
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