Bug/607 fidelity statevector kernel cannot be pickled #778

oscar-wallis · 2024-02-26T15:55:00Z

Summary

Fixes #607 where FidelityStatevectorKernel was throwing an error when pickled due to a known pickling error with functools.lru_cache. This has been fixed by adding a self.cache_size attribute and custom getstate and setstate functions used when pickling and unpickling.

Details and comments

When pickled the lru_cache is set to None in the object dict. When unpicked the cache is re-initiliased with the new cache_size variable and getstate and setstate. Unit tests added to check object pickling and unpickling as well as manually using getstate and setstate to initialise a FidelityStatevectorKernel. Passed all other unit tests.

Added a new param to store cache size and a custom __getstate__ and __setstate__ to handle removing and re-initliasing the lru cache during pickle/unpickling respectively.

…orKernel-cannot-be-pickled

https://github.com/OkuyanBoga/hc-qiskit-machine-learning into bug/607---FidelityStatevectorKernel-cannot-be-pickled

woodsp-ibm

Normally if we have a bug that does not show up in the test cases it suggests we need one to test that aspects for better coverage and to make sure it things stay working. I think a test case that pickles and unpickles is needed.

qiskit_machine_learning/kernels/fidelity_statevector_kernel.py

releasenotes/notes/fix-fid_statevector_kernel-pickling-b7fa2b13a15ec9c6.yaml

oscar-wallis · 2024-02-26T16:59:29Z

Normally if we have a bug that does not show up in the test cases it suggests we need one to test that aspects for better coverage and to make sure it things stay working. I think a test case that pickles and unpickles is needed.

Does that mean a test is needed where we pickle and unpickle all qml objects?

woodsp-ibm · 2024-02-26T17:05:33Z

Does that mean a test is needed where we pickle and unpickle all qml objects?

Seems like a lot of objects! This is much more an edge case I guess due to lru_cache so I would leave it at just a test for this object - makes sure it works in CI over the platforms and continues to do so.

coveralls · 2024-02-26T17:33:16Z

Pull Request Test Coverage Report for Build 8098671846

Details

9 of 9 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.03%) to 92.636%

Totals
Change from base Build 8098398840:	0.03%
Covered Lines:	1887
Relevant Lines:	2037

💛 - Coveralls

oscar-wallis · 2024-02-27T15:36:50Z

Added unit testing and edited release notes.

test/kernels/test_fidelity_statevector_kernel.py

declanmillar

Thanks very much for addressing this issue! I just have a few minor suggestions.

releasenotes/notes/fix-fid_statevector_kernel-pickling-b7fa2b13a15ec9c6.yaml

.gitignore

test/kernels/test_fidelity_statevector_kernel.py

qiskit_machine_learning/kernels/fidelity_statevector_kernel.py

test/kernels/test_fidelity_statevector_kernel.py

…3a15ec9c6.yaml Co-authored-by: Declan Millar <[email protected]>

Co-authored-by: Declan Millar <[email protected]>

oscar-wallis · 2024-02-29T15:21:16Z

Thanks very much for addressing this issue! I just have a few minor suggestions.

I appreciate the suggestions! I'm surprised some of them weren't picked up by CI. All the changes were approved.

…e-pickled

declanmillar · 2024-02-29T15:26:50Z

You'll need to add from typing import Any to fidelity_statevector_kernel.py, but, unfortunately, I'm not allowed to make that suggestion as it's in an unedited code block.

oscar-wallis · 2024-02-29T15:31:56Z

from typing import Any

Can do, why does it need to be added? - for the dict, got it

adekusar-drl

Looks good to me. Thank you.

* Made fidelity_statevector_kernel picklable Added a new param to store cache size and a custom __getstate__ and __setstate__ to handle removing and re-initliasing the lru cache during pickle/unpickling respectively. * updated notes * name changes * spell corrections * Updated description * Added unittest for pickling * Spelling changes * Making error messages clearer * Spelling -_- * Update releasenotes/notes/fix-fid_statevector_kernel-pickling-b7fa2b13a15ec9c6.yaml Co-authored-by: Declan Millar <[email protected]> * Update .gitignore Co-authored-by: Declan Millar <[email protected]> * Update test/kernels/test_fidelity_statevector_kernel.py Co-authored-by: Declan Millar <[email protected]> * Update qiskit_machine_learning/kernels/fidelity_statevector_kernel.py Co-authored-by: Declan Millar <[email protected]> * Update test/kernels/test_fidelity_statevector_kernel.py Co-authored-by: Declan Millar <[email protected]> * Update qiskit_machine_learning/kernels/fidelity_statevector_kernel.py Co-authored-by: Declan Millar <[email protected]> * Added Any class --------- Co-authored-by: M. Emre Sahin <[email protected]> Co-authored-by: Declan Millar <[email protected]> Co-authored-by: Anton Dekusar <[email protected]> (cherry picked from commit c59063a)

* Made fidelity_statevector_kernel picklable Added a new param to store cache size and a custom __getstate__ and __setstate__ to handle removing and re-initliasing the lru cache during pickle/unpickling respectively. * updated notes * name changes * spell corrections * Updated description * Added unittest for pickling * Spelling changes * Making error messages clearer * Spelling -_- * Update releasenotes/notes/fix-fid_statevector_kernel-pickling-b7fa2b13a15ec9c6.yaml Co-authored-by: Declan Millar <[email protected]> * Update .gitignore Co-authored-by: Declan Millar <[email protected]> * Update test/kernels/test_fidelity_statevector_kernel.py Co-authored-by: Declan Millar <[email protected]> * Update qiskit_machine_learning/kernels/fidelity_statevector_kernel.py Co-authored-by: Declan Millar <[email protected]> * Update test/kernels/test_fidelity_statevector_kernel.py Co-authored-by: Declan Millar <[email protected]> * Update qiskit_machine_learning/kernels/fidelity_statevector_kernel.py Co-authored-by: Declan Millar <[email protected]> * Added Any class --------- Co-authored-by: M. Emre Sahin <[email protected]> Co-authored-by: Declan Millar <[email protected]> Co-authored-by: Anton Dekusar <[email protected]> (cherry picked from commit c59063a) Co-authored-by: oscar-wallis <[email protected]>

oscar-wallis and others added 5 commits February 20, 2024 14:21

Made fidelity_statevector_kernel picklable

278fb45

Added a new param to store cache size and a custom __getstate__ and __setstate__ to handle removing and re-initliasing the lru cache during pickle/unpickling respectively.

updated notes

2e42166

Merge branch 'qiskit-community:main' into bug/607---FidelityStatevect…

171ea50

…orKernel-cannot-be-pickled

name changes

f3dc9f7

Merge branch 'bug/607---FidelityStatevectorKernel-cannot-be-pickled' of

d223d95

https://github.com/OkuyanBoga/hc-qiskit-machine-learning into bug/607---FidelityStatevectorKernel-cannot-be-pickled

oscar-wallis requested review from woodsp-ibm, adekusar-drl, smens, edoaltamura, OkuyanBoga and Benjamin-Symons as code owners February 26, 2024 15:55

spell corrections

67ae030

woodsp-ibm reviewed Feb 26, 2024

View reviewed changes

qiskit_machine_learning/kernels/fidelity_statevector_kernel.py Outdated Show resolved Hide resolved

releasenotes/notes/fix-fid_statevector_kernel-pickling-b7fa2b13a15ec9c6.yaml Outdated Show resolved Hide resolved

Updated description

cd07e9c

oscar-wallis marked this pull request as draft February 26, 2024 17:24

Added unittest for pickling

c4f8ce7

oscar-wallis marked this pull request as ready for review February 27, 2024 15:49

Spelling changes

683c424

OkuyanBoga reviewed Feb 29, 2024

View reviewed changes

test/kernels/test_fidelity_statevector_kernel.py Outdated Show resolved Hide resolved

oscar-wallis added 2 commits February 29, 2024 10:54

Making error messages clearer

83a00d3

Spelling -_-

173b692

declanmillar suggested changes Feb 29, 2024

View reviewed changes

oscar-wallis and others added 4 commits February 29, 2024 15:16

Update releasenotes/notes/fix-fid_statevector_kernel-pickling-b7fa2b1…

6da44ae

…3a15ec9c6.yaml Co-authored-by: Declan Millar <[email protected]>

Update .gitignore

bb072fc

Co-authored-by: Declan Millar <[email protected]>

Update test/kernels/test_fidelity_statevector_kernel.py

f78546a

Co-authored-by: Declan Millar <[email protected]>

Update qiskit_machine_learning/kernels/fidelity_statevector_kernel.py

3c0b7d2

Co-authored-by: Declan Millar <[email protected]>

oscar-wallis and others added 2 commits February 29, 2024 15:19

Update test/kernels/test_fidelity_statevector_kernel.py

aae295d

Co-authored-by: Declan Millar <[email protected]>

Update qiskit_machine_learning/kernels/fidelity_statevector_kernel.py

3c69245

Co-authored-by: Declan Millar <[email protected]>

Merge branch 'main' into bug/607---FidelityStatevectorKernel-cannot-b…

87bd94f

…e-pickled

Added Any class

a66ce2b

adekusar-drl approved these changes Feb 29, 2024

View reviewed changes

edoaltamura approved these changes Feb 29, 2024

View reviewed changes

adekusar-drl added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Feb 29, 2024

oscar-wallis merged commit c59063a into qiskit-community:main Feb 29, 2024
16 checks passed

mergify bot mentioned this pull request Feb 29, 2024

Bug/607 fidelity statevector kernel cannot be pickled (backport #778) #782

Merged

oscar-wallis deleted the bug/607---FidelityStatevectorKernel-cannot-be-pickled branch March 11, 2024 15:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug/607 fidelity statevector kernel cannot be pickled #778

Bug/607 fidelity statevector kernel cannot be pickled #778

oscar-wallis commented Feb 26, 2024 •

edited

Loading

woodsp-ibm left a comment

oscar-wallis commented Feb 26, 2024

woodsp-ibm commented Feb 26, 2024

coveralls commented Feb 26, 2024 •

edited

Loading

oscar-wallis commented Feb 27, 2024

declanmillar left a comment

oscar-wallis commented Feb 29, 2024

declanmillar commented Feb 29, 2024

oscar-wallis commented Feb 29, 2024 •

edited

Loading

adekusar-drl left a comment

Bug/607 fidelity statevector kernel cannot be pickled #778

Bug/607 fidelity statevector kernel cannot be pickled #778

Conversation

oscar-wallis commented Feb 26, 2024 • edited Loading

Summary

Details and comments

woodsp-ibm left a comment

Choose a reason for hiding this comment

oscar-wallis commented Feb 26, 2024

woodsp-ibm commented Feb 26, 2024

coveralls commented Feb 26, 2024 • edited Loading

Pull Request Test Coverage Report for Build 8098671846

Details

💛 - Coveralls

oscar-wallis commented Feb 27, 2024

declanmillar left a comment

Choose a reason for hiding this comment

oscar-wallis commented Feb 29, 2024

declanmillar commented Feb 29, 2024

oscar-wallis commented Feb 29, 2024 • edited Loading

adekusar-drl left a comment

Choose a reason for hiding this comment

oscar-wallis commented Feb 26, 2024 •

edited

Loading

coveralls commented Feb 26, 2024 •

edited

Loading

oscar-wallis commented Feb 29, 2024 •

edited

Loading