Bug fix on QNSPSA #9483

luciacuervovalor · 2023-01-30T13:19:51Z

Old syntax was giving type error when dividing int by float

Summary

Minor syntax change because the old one gave random type errors .

Details and comments

Very minor change, no important details

Old syntax was giving type error when dividing int by float

CLAassistant · 2023-01-30T13:19:56Z

All committers have signed the CLA.

CLAassistant · 2023-01-30T13:19:56Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

qiskit-bot · 2023-01-30T13:20:01Z

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

@Qiskit/terra-core
@manoelmarques
@woodsp-ibm

Cryoris · 2023-01-30T13:31:06Z

Hi @luciacuervovalor, could you add a small test and a bugfix release note? 🙂

Cryoris · 2023-02-01T10:19:35Z

qiskit/algorithms/optimizers/qnspsa.py

@@ -228,9 +228,10 @@ def _point_sample(self, loss, x, eps, delta1, delta2):
        gradient_estimate = (loss_values[0] - loss_values[1]) / (2 * eps) * delta1

        # compute the preconditioner point estimate
+        fidelity_values = [float(f) for f in fidelity_values]


Are the casts to float still required now that we explicitly use diff = diff / thing instead of diff /= thing?

I think so because it failed again with diff = diff / thing with the same error

releasenotes/notes/qnspsa-float-bug-fix-4035f7e1eb61dec2.yaml

Co-authored-by: Julien Gacon <gaconju@gmail.com>

…erra into qnspsa_fix_bug

jakelishman · 2023-02-01T14:07:38Z

test/python/algorithms/optimizers/test_spsa.py

+    def test_point_sample(self):
+        """Test point sample function in QNSPSA"""
+
+        def fidelity(x, y):


pylint's moaning about the unused y argument. Given this is just a stub function, you can make it shut up by giving the second argument a name starting with an underscore, like _y - that means "explicitly ignored".

done, thanks!

Oh man, and now Pylint's complaining that that's not a valid name... I'm not a fan of these pylint rules at all.

At any rate, you can put a comment just under the docstring of test_point_sample that says # pylint: disable=invalid-name or # pylint: disable=unused-argument as appropriate (depending on which name for the argument you prefer using).

coveralls · 2023-02-01T14:59:48Z

Pull Request Test Coverage Report for Build 4072796493

3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
111 unchanged lines in 8 files lost coverage.
Overall coverage decreased (-0.02%) to 85.245%

Files with Coverage Reduction	New Missed Lines	%
qiskit/transpiler/passes/optimization/echo_rzx_weyl_decomposition.py	2	97.01%
qiskit/transpiler/passmanager.py	2	93.65%
qiskit/pulse/library/waveform.py	3	91.67%
qiskit/algorithms/state_fidelities/compute_uncompute.py	4	93.75%
qiskit/circuit/register.py	6	93.94%
qiskit/pulse/calibration_entries.py	13	87.88%
qiskit/transpiler/passes/calibration/rzx_builder.py	22	87.06%
qiskit/visualization/pass_manager_visualization.py	59	17.53%

Totals
Change from base Build 4028718413:	-0.02%
Covered Lines:	67109
Relevant Lines:	78725

💛 - Coveralls

The name of variable "_y" must have this form because the argument is unused

ElePT

The change LGTM! Thanks @luciacuervovalor :)

* Bug fix Old syntax was giving type error when dividing int by float * Bug fix * Convert to float to avoid type error * Add unit test * Add release note * Update releasenotes/notes/qnspsa-float-bug-fix-4035f7e1eb61dec2.yaml Co-authored-by: Julien Gacon <gaconju@gmail.com> * Ran black * Make unused argument explicitly ignored * Change to np array syntax * Pylint disable invalid name The name of variable "_y" must have this form because the argument is unused * Make black --------- Co-authored-by: Julien Gacon <gaconju@gmail.com> Co-authored-by: ElePT <epenatap@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 2f49b5a)

* Bug fix Old syntax was giving type error when dividing int by float * Bug fix * Convert to float to avoid type error * Add unit test * Add release note * Update releasenotes/notes/qnspsa-float-bug-fix-4035f7e1eb61dec2.yaml Co-authored-by: Julien Gacon <gaconju@gmail.com> * Ran black * Make unused argument explicitly ignored * Change to np array syntax * Pylint disable invalid name The name of variable "_y" must have this form because the argument is unused * Make black --------- Co-authored-by: Julien Gacon <gaconju@gmail.com> Co-authored-by: ElePT <epenatap@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 2f49b5a) Co-authored-by: luciacuervovalor <67261900+luciacuervovalor@users.noreply.github.com> Co-authored-by: Jake Lishman <jake.lishman@ibm.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Bug fix Old syntax was giving type error when dividing int by float * Bug fix * Convert to float to avoid type error * Add unit test * Add release note * Update releasenotes/notes/qnspsa-float-bug-fix-4035f7e1eb61dec2.yaml Co-authored-by: Julien Gacon <gaconju@gmail.com> * Ran black * Make unused argument explicitly ignored * Change to np array syntax * Pylint disable invalid name The name of variable "_y" must have this form because the argument is unused * Make black --------- Co-authored-by: Julien Gacon <gaconju@gmail.com> Co-authored-by: ElePT <epenatap@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Bug fix

5858cf5

Old syntax was giving type error when dividing int by float

luciacuervovalor requested review from a team, manoelmarques, woodsp-ibm and ElePT as code owners January 30, 2023 13:19

qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Jan 30, 2023

luciacuervovalor added 4 commits January 31, 2023 11:00

Bug fix

d50455f

Convert to float to avoid type error

032821f

Add unit test

a1e72f2

Add release note

db9252e

Cryoris reviewed Feb 1, 2023

View reviewed changes

luciacuervovalor and others added 3 commits February 1, 2023 13:14

Update releasenotes/notes/qnspsa-float-bug-fix-4035f7e1eb61dec2.yaml

246d8a5

Co-authored-by: Julien Gacon <gaconju@gmail.com>

Ran black

32afd3b

Merge branch 'qnspsa_fix_bug' of github.com:luciacuervovalor/qiskit-t…

df36a71

…erra into qnspsa_fix_bug

jakelishman reviewed Feb 1, 2023

View reviewed changes

Make unused argument explicitly ignored

16d2a9d

Cryoris added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Feb 1, 2023

luciacuervovalor added 2 commits February 1, 2023 16:08

Change to np array syntax

b89370b

Pylint disable invalid name

2368ac8

The name of variable "_y" must have this form because the argument is unused

jakelishman added this to the 0.23.2 milestone Feb 1, 2023

Make black

79da909

ElePT approved these changes Feb 2, 2023

View reviewed changes

ElePT added the automerge label Feb 2, 2023

Merge branch 'main' into qnspsa_fix_bug

9ca84bd

mergify bot merged commit 2f49b5a into Qiskit:main Feb 2, 2023

mergify bot mentioned this pull request Feb 2, 2023

Bug fix on QNSPSA (backport #9483) #9520

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug fix on QNSPSA #9483

Bug fix on QNSPSA #9483

luciacuervovalor commented Jan 30, 2023

CLAassistant commented Jan 30, 2023 •

edited

Loading

CLAassistant commented Jan 30, 2023

qiskit-bot commented Jan 30, 2023

Cryoris commented Jan 30, 2023

Cryoris Feb 1, 2023

luciacuervovalor Feb 1, 2023

jakelishman Feb 1, 2023

luciacuervovalor Feb 1, 2023

jakelishman Feb 1, 2023

coveralls commented Feb 1, 2023 •

edited

Loading

ElePT left a comment

Bug fix on QNSPSA #9483

Bug fix on QNSPSA #9483

Conversation

luciacuervovalor commented Jan 30, 2023

Summary

Details and comments

CLAassistant commented Jan 30, 2023 • edited Loading

CLAassistant commented Jan 30, 2023

qiskit-bot commented Jan 30, 2023

Cryoris commented Jan 30, 2023

Cryoris Feb 1, 2023

Choose a reason for hiding this comment

luciacuervovalor Feb 1, 2023

Choose a reason for hiding this comment

jakelishman Feb 1, 2023

Choose a reason for hiding this comment

luciacuervovalor Feb 1, 2023

Choose a reason for hiding this comment

jakelishman Feb 1, 2023

Choose a reason for hiding this comment

coveralls commented Feb 1, 2023 • edited Loading

Pull Request Test Coverage Report for Build 4072796493

💛 - Coveralls

ElePT left a comment

Choose a reason for hiding this comment

CLAassistant commented Jan 30, 2023 •

edited

Loading

coveralls commented Feb 1, 2023 •

edited

Loading