Fix #44: reject non-numeric qubit and cost ceilings instead of treating them as zero - #89
Open
corgab wants to merge 2 commits into
Open
Fix #44: reject non-numeric qubit and cost ceilings instead of treating them as zero#89corgab wants to merge 2 commits into
corgab wants to merge 2 commits into
Conversation
…hem as zero max_qubits and max_cost_per_run come straight from env() as strings, and (int) "abc" is 0 in PHP, so a typo in .env silently became a ceiling of zero that rejected every circuit with a message blaming the circuit. The drivers now read both options through validating helpers: blank stays "no ceiling", a positive integer or a non-negative number is accepted (numeric strings included), and anything else throws InvalidDriverConfigException naming the key and the value it got. Closes #44
positiveIntegerConfig() and nonNegativeNumberConfig() now share a single filteredConfig() that handles the blank check, the boolean exclusion and the filter_var call, so the two readers differ only in filter and range. validateCircuits() resolves max_qubits once per call instead of once per circuit and documents the InvalidDriverConfigException it can raise; the README lists every rejected shape, not only non-numeric strings.
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
1 participant
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.
Summary
max_qubitsandmax_cost_per_runare read fromenv()without a cast, and the guards hard-cast them at the point of use:(int) $ceilinginassertWithinQubitCeiling(),(float) $ceilinginassertWithinCostCeiling().(int) "abc"is0, so a typo or an unset shell variable in.envturned into a ceiling of zero that rejected every circuit with anInvalidCircuitExceptionblaming the circuit, not the configuration.Plan
Validate at the point of use rather than in
config/aether.php, so a published config that still carries the rawenv()call is covered too, and keep the blank-means-unlimited behaviour intact. This is the targeted fix for the symptom; #66 (typed driver configuration) remains the place for the broader refactor and can absorb these helpers.Changes
AbstractQuantumDriver: a privatefilteredConfig($key, $filter, $options, $expected)does the blank check (null/''mean "no ceiling"), refuses booleans (filter_var(true, FILTER_VALIDATE_INT)would yield 1) and runsfilter_var; on rejection it throwsInvalidDriverConfigException::invalidValue()naming the driver, key, value and expected shape. Two protected readers sit on top:positiveIntegerConfig()(FILTER_VALIDATE_INT,min_range1) andnonNegativeNumberConfig()(FILTER_VALIDATE_FLOAT,min_range0). Numeric strings are accepted.validateCircuits()resolvesmax_qubitsonce per call and passes the integer toassertWithinQubitCeiling(); its docblock now listsInvalidDriverConfigException.AwsBraketDriver::assertWithinCostCeiling()usesnonNegativeNumberConfig(); the estimate comparison no longer casts.InvalidDriverConfigException::invalidValue()(same signature and message as the one introduced in Fix #69: derive synchronous safety from the device ARN by default #74, so the two branches merge cleanly).config/aether.php: one comment on each option stating the accepted shapes. README "Qubit Ceiling" and "Cost Estimation": one sentence each listing what is rejected (typos,0, negatives, decimals for qubits; non-numeric or negative for cost).No new dependencies, no behaviour change for valid or blank values.
Tests
AbstractQuantumDriverTest: dataset ('abc','2.5','0',-3,true) throwsInvalidDriverConfigExceptionmentioning[max_qubits]without touching the bridge; a numeric string'4'is still honoured as the ceiling.AwsBraketDriverTest: dataset ('abc','-1',true) throws mentioning[max_cost_per_run]; a numeric string'0.50'is still honoured.Local: pint passed, pest 801 tests passed.
Closes #44