Skip to content

fix: resolve measurement destination index before branching#391

Open
AJ-comp wants to merge 1 commit into
amazon-braket:mainfrom
AJ-comp:fix/branched-measurement-indexed-destination
Open

fix: resolve measurement destination index before branching#391
AJ-comp wants to merge 1 commit into
amazon-braket:mainfrom
AJ-comp:fix/branched-measurement-indexed-destination

Conversation

@AJ-comp

@AJ-comp AJ-comp commented Jul 15, 2026

Copy link
Copy Markdown

Issue #, if available:

Fixes #390.

Description of changes:

b[i] = measure q[j] crashes with AttributeError: 'Identifier' object has no attribute 'value' once a later condition promotes the deferred measurement to branched execution. A literal index works; const, identifier and range destinations do not.

The index is already resolved before it is needed. It is then dropped, and recomputed by reading .value off the raw AST node — which only an IntegerLiteral has.

interpreter.py           resolves node.target.indices, passes it as classical_targets  <- correct value
_branch_measurement      receives it, uses it correctly for _circuit.add_measure(...)
                         └─ does not forward it to _update_classical_from_measurement  <- dropped here
_update_indexed_target   re-derives the index from the raw AST
_resolve_index           indices[0][0].value   <- Identifier has .name, not .value  -> crash

This is not a scoping problem: const int k = 0 is still perfectly in scope when the crash happens — it is simply never looked up. _resolve_index reads the attribute straight off the node.

Resolving it lazily at replay time would not be a general fix either, though:

destination index resolvable when the measurement is replayed?
const int k = 0 yes
for loop variable i no — KeyError: 'Undefined key: i', the loop has exited

So the fix is to forward classical_targets and use it. That is what the non-branched path already does: _flush_pending_mcm_targets discards the destination (_mcm_dest) and uses the resolved indices. This makes branched execution agree with non-branched execution.

Zipping the resolved indices with the measured qubits also fixes range destinations (b[0:1] = measure q[0:1]), previously 'RangeDefinition' object has no attribute 'value'. _resolve_index had no other caller and is removed.

Why this is safe: _update_indexed_target is only reached when the destination is an IndexedIdentifier, and interpreter.py always populates classical_targets in that case. The one caller that passes a destination without targets — _handle_measure_ff — uses a plain Identifier, which routes to _update_identifier_target instead.

Testing done:

tox passes — 1188 unit tests, linters clean.

Branched execution now matches non-branched:

destination non-branched branched, before branched, after
r[0] literal '1' '1' '1'
r[k], const int k = 0 '1' crash '1'
r[i], for int i in [0:1] '10' crash '10'
r[0:1] = measure q '10' crash '10'

Added test_branched_measurement_into_indexed_destination (literal / const / non-const identifier), test_branched_measurement_into_loop_variable_index and test_branched_measurement_into_range_destination. Four of the five fail against the unpatched source; the literal row is the regression guard and passes either way.

Feed-forward works end to end rather than merely not crashing: with the branch made deterministic, r[k] = measure q[0]; if (r[0] == true) { x q[1]; } gives {'11': 10}.

Also removes the xfail from test_5_2_for_loop_operations_with_branching ("Interpreter gap: branched condition BinaryExpression not fully resolved"), which this fixes.

Out of scope: r[1-1] = measure q[0] fails on the branched and non-branched paths, because interpreter.py never evaluates a destination index that is a BinaryExpression. That is a separate gap, and the two paths agree there.

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

Tests

  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have checked that my tests are not configured for a specific region or account (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

A measurement whose classical destination is indexed (`b[i] = measure q[j]`) crashed with
`AttributeError: 'Identifier' object has no attribute 'value'` whenever a later condition
promoted the deferred measurement to branched execution.

The interpreter already resolves the destination index and passes it to `add_measure` as
`classical_targets`, and `_branch_measurement` receives it — but it was not forwarded to
`_update_classical_from_measurement`, so `_update_indexed_target` re-derived the index from the
raw AST via `_resolve_index`, which assumed an `IntegerLiteral`. Lazy resolution cannot work in
general: a `for` loop variable is already out of scope by the time the pending measurement is
replayed.

Forward `classical_targets` and use it. This matches the non-branched path, which already
destructures the destination away and uses the resolved indices, and it makes branched execution
agree with non-branched execution for `const`, identifier and range destinations. `_resolve_index`
had no other caller and is removed. Iterating the resolved indices alongside the measured qubits
also fixes range destinations (`b[0:1] = measure q[0:1]`), which previously raised
`'RangeDefinition' object has no attribute 'value'`.

Removes the xfail from test_5_2_for_loop_operations_with_branching, which this fixes.
@AJ-comp AJ-comp requested a review from a team as a code owner July 15, 2026 20:44
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (f454cee) to head (cade13e).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #391   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           49        49           
  Lines         4927      4924    -3     
  Branches       569       570    +1     
=========================================
- Hits          4927      4924    -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LocalSimulator crash: 'Identifier' object has no attribute 'value' when a measurement's classical destination uses a non-literal index

1 participant