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

QKT deepcopies kernel internally, fails with runtime primitives #600

Closed
ElePT opened this issue Apr 12, 2023 · 4 comments
Closed

QKT deepcopies kernel internally, fails with runtime primitives #600

ElePT opened this issue Apr 12, 2023 · 4 comments
Assignees
Labels
type: bug 🐞 Something isn't working

Comments

@ElePT
Copy link
Collaborator

ElePT commented Apr 12, 2023

Environment

  • Qiskit Machine Learning version: 0.6.0
  • Python version: all
  • Operating system: all

What is happening?

On line 202, the quantum kernel trainer performs: output_kernel = copy.deepcopy(self._quantum_kernel), which is incompatible with the runtime primitives, as they do not support deepcopy. If you try to run it, a recursion error is raised (this does not happen with the Terra reference or Aer primitives).

How can we reproduce the issue?

See the original issue: Qiskit/qiskit-ibm-runtime#745

What should happen?

The QKT class should work seamlessly for all primitives.

Any suggestions?

I think it's worth revising whether we need to copy the kernel, or whether we need to return a kernel at all.

  • One possibility would be for the QKT class to return the trained paremeters and have the user deal with them as they wish (downside -> breaks current interface)
  • Another possibility would be to develop a more sophisticated kernel.copy that creates a new kernel without copying the primitive
  • Finally, we could have the QKT modify the original kernel object
@xaviervasques
Copy link

Hi @ElePT,
Have you been able to solve the problem ?
Thank you !

@adekusar-drl
Copy link
Collaborator

I'm looking into the issue. Sorry for the delay

@ElePT
Copy link
Collaborator Author

ElePT commented Jun 14, 2023

@adekusar-drl I believe this PR should fix the issue: Qiskit/qiskit-ibm-runtime#902

oscar-wallis added a commit to OkuyanBoga/hc-qiskit-machine-learning that referenced this issue Feb 9, 2024
oscar-wallis added a commit to OkuyanBoga/hc-qiskit-machine-learning that referenced this issue Feb 16, 2024
oscar-wallis added a commit that referenced this issue Feb 16, 2024
@oscar-wallis
Copy link
Collaborator

Hiya, this issue should be fixed within the springv1 Hartree centre qml PR.

OkuyanBoga added a commit that referenced this issue Feb 29, 2024
…ntum kernel trainer fixing #701 and #600 (#772)

* Added an option for num_circuits per job for kernels to fix #701

* Updated documentation and format the style.

* Removed deepcopy dependency in quantum_kernel_trainer.py

* Added release notes

* quick fix for spell test

* Added unit tests for max_circuits_per_job

* Update fix-701-max_circuits_per_job-and-600-deepcopy-dependency-e6eda2e5b986c1be.yaml

Small release note bugfix

* Update fix-701-max_circuits_per_job-and-600-deepcopy-dependency-e6eda2e5b986c1be.yaml

* Minor modifications for the unit test

* Removed copy of TrainableKernel

---------

Co-authored-by: oscar-wallis <oscar.wallis@outlook.com>
mergify bot pushed a commit that referenced this issue Feb 29, 2024
…ntum kernel trainer fixing #701 and #600 (#772)

* Added an option for num_circuits per job for kernels to fix #701

* Updated documentation and format the style.

* Removed deepcopy dependency in quantum_kernel_trainer.py

* Added release notes

* quick fix for spell test

* Added unit tests for max_circuits_per_job

* Update fix-701-max_circuits_per_job-and-600-deepcopy-dependency-e6eda2e5b986c1be.yaml

Small release note bugfix

* Update fix-701-max_circuits_per_job-and-600-deepcopy-dependency-e6eda2e5b986c1be.yaml

* Minor modifications for the unit test

* Removed copy of TrainableKernel

---------

Co-authored-by: oscar-wallis <oscar.wallis@outlook.com>
(cherry picked from commit 2f49e9e)
adekusar-drl pushed a commit that referenced this issue Feb 29, 2024
…ntum kernel trainer fixing #701 and #600 (#772) (#780)

* Added an option for num_circuits per job for kernels to fix #701

* Updated documentation and format the style.

* Removed deepcopy dependency in quantum_kernel_trainer.py

* Added release notes

* quick fix for spell test

* Added unit tests for max_circuits_per_job

* Update fix-701-max_circuits_per_job-and-600-deepcopy-dependency-e6eda2e5b986c1be.yaml

Small release note bugfix

* Update fix-701-max_circuits_per_job-and-600-deepcopy-dependency-e6eda2e5b986c1be.yaml

* Minor modifications for the unit test

* Removed copy of TrainableKernel

---------

Co-authored-by: oscar-wallis <oscar.wallis@outlook.com>
(cherry picked from commit 2f49e9e)

Co-authored-by: M. Emre Sahin <40424147+OkuyanBoga@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐞 Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants