【代码贡献】Fix SPSA optimizer ignoring parameter bounds - #41
Open
mnn31 wants to merge 1 commit into
Open
Conversation
_check_bounds returned None when one (min, max) pair was given per variable, so _jail_inside treated it as no bounds and never clipped. The single-pair branch multiplied the bounds array by d instead of repeating the pair d times.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to #13.
Problem
spsa_minimizesilently ignores theboundsargument in the normal case, andcorrupts it in the other case. The first path is what
QAOA.run(optimizer='SPSA')hits, since it always passes one pair per parameter; the second is reachable
through the documented
spsa_minimize(..., bounds=[(min, max)])public API.Root cause
pyqpanda_alg/QAOA/spsa.py,_check_bounds(lines 17-33).(min, max)pair per variable,len(bounds) == d: the branch validatesthe dimension and then falls off the end of the function, so it returns
None._jail_insidereadsNoneas "no bounds" and never clips.len(bounds) == 1:return bounds * dmultiplies the numpy array element-wise by
dinstead of repeating the rowdtimes._check_bounds(3, [(0, 1)])gives[[0, 3]], which is the wronginterval and the wrong shape, so the later
np.clipbroadcasts one boguspair over every variable.
Fix
Return the validated array in the per-variable branch, and use
np.tileforthe single-pair branch. The empty-bounds warning and both existing
ValueError/IndexErrorchecks are unchanged.Verification
before
after
Same thing through
QAOA.run(optimizer='SPSA')withgamma_bounds = beta_bounds = [(0.0, 0.5)]: ondevelopthe returnedparameters come back outside the bounds; with this change they stay inside.
With the default (unbounded) settings the result is bit-identical to
develop.Tests
5 cases added to
test/QAOA/Test_spsa_minimize.py: bounds respected end to endfor per-variable bounds and for a single tiled pair,
_check_boundsreturningthe array instead of
None, single-pair tiling, and the existing validationerrors. Seeded, deterministic. 4 of the 5 fail on
developand pass with thischange.
The gradient line in
spsa_minimizedivides byxp - xmrather than2 * ck * delta. Withdeltain{-1, 1}and no clipping these areidentical, and when clipping is active the current form is a finite difference
over the points actually evaluated, so I left it alone. Degenerate
lo == hibounds make that denominator zero; that is a property of theexisting gradient line, not of this change, so it is out of scope here too.