Skip to content

Make tests in test_noisy_density_matrix.py use the BranchSelector feature#425

Open
tibisabau wants to merge 21 commits intoTeamGraphix:masterfrom
tibisabau:416-makes-backend-tests-deterministic
Open

Make tests in test_noisy_density_matrix.py use the BranchSelector feature#425
tibisabau wants to merge 21 commits intoTeamGraphix:masterfrom
tibisabau:416-makes-backend-tests-deterministic

Conversation

@tibisabau
Copy link
Contributor

Before submitting, please check the following:

  • Make sure you have tests for the new code and that test passes (run nox)
  • If applicable, add a line to the [unreleased] part of CHANGELOG.md, following keep-a-changelog.
  • Format added code by ruff
    • See CONTRIBUTING.md for more details
  • Make sure the checks (github actions) pass.

Then, please fill in below:

Context (if applicable):

The noisy density matrix backend tests in test_noisy_density_matrix.py were non-deterministic because they relied on random measurement outcomes. These tests used or conditions to check multiple possible outcomes, making them fail intermittently. The BranchSelector feature (introduced in PR #300) allows fixing measurement outcomes deterministically.

Description of the change:

Modified all affected tests in test_noisy_density_matrix.py to use BranchSelector (ConstBranchSelector and FixedBranchSelector) to fix measurement outcomes, making the tests fully deterministic. Updated test logic based on understanding that noise channels are applied unconditionally after correction commands in the depolarising noise model.

Related issue:

Fixes #416

@codecov
Copy link

codecov bot commented Feb 6, 2026

Codecov Report

❌ Patch coverage is 88.57143% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.72%. Comparing base (8c0fcef) to head (b8c62a7).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
graphix/pattern.py 66.66% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #425      +/-   ##
==========================================
+ Coverage   86.71%   86.72%   +0.01%     
==========================================
  Files          44       44              
  Lines        6156     6163       +7     
==========================================
+ Hits         5338     5345       +7     
  Misses        818      818              

☔ View full report in Codecov by Sentry.
📢 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.

thierry-martinez and others added 5 commits February 6, 2026 09:10
* Update CHANGELOG

* Remove trailing spaces

* Update graphix-qasm-parser constraint
Co-authored-by: thierry-martinez <thierry.martinez@inria.fr>
Co-authored-by: thierry-martinez <thierry.martinez@inria.fr>
Co-authored-by: thierry-martinez <thierry.martinez@inria.fr>
Co-authored-by: thierry-martinez <thierry.martinez@inria.fr>
@thierry-martinez
Copy link
Collaborator

@tibisabau Thank you for your last commits. I pointed just the first occurrences of each problem, but you need to solve all the other occurrences for ruff and mypy to be happy. You can run ruff and mypy (and even nox) locally to check that all CI checks will pass before pushing.

Comment on lines 366 to 375
# M(0) determines Z, M(1) determines X
results: dict[int, Outcome] = {}
cmd_count = 0
for cmd in rzpattern:
if cmd.kind.name == "M":
if cmd_count == 0:
results[cast("M", cmd).node] = outcome_z
elif cmd_count == 1:
results[cast("M", cmd).node] = outcome_x
cmd_count += 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My previous comment (#425 (comment)) applies here and lines 410 and below as well: in particular, we don't need to use cast here.

)

def test_noisy_z_rz(self, fx_rng: Generator) -> None:
@pytest.mark.parametrize(("outcome_z", "outcome_x"), [(0, 0), (0, 1), (1, 0), (1, 1)])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For homogeneity, I would prefer if we do the same here as for line 324 (and this applies to line 398 and 442 too).

tibisabau and others added 3 commits February 6, 2026 17:24
…aphix#427)

Bumps the python-packages group with 1 update: [ruff](https://github.com/astral-sh/ruff).


Updates `ruff` from 0.14.14 to 0.15.0
- [Release notes](https://github.com/astral-sh/ruff/releases)
- [Changelog](https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md)
- [Commits](astral-sh/ruff@0.14.14...0.15.0)

---
updated-dependencies:
- dependency-name: ruff
  dependency-version: 0.15.0
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: python-packages
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@mgarnier59
Copy link
Contributor

@tibisabau thanks for your PR! While checking it, we realised that the behaviour in the test was the one we wanted but not the one taht was implemented. Son instead of making the tests fit the implementation, we'll modify the implementation see #428 for more details.

Thanks again and keep an eye on further developments!

thierry-martinez and others added 11 commits February 12, 2026 10:51
…amGraphix#431)

This commit installs graphix after the reverse dependency to ensure
that the test uses the current graphix codebase even if another
version is pinned in the reverse dependency.
…amGraphix#429)

* fix noise conditionality and extract it

* finalizing refactor and removing test

* lint

* implement Thierry's comments

* move default `check_domain` to `MeasureMethod`

* Fix `ApplyNoise` docstring

* Update noise_model.py

* Update noise_model.py

* double backticks for doc

* Update CHANGELOG.md
Co-authored-by: thierry-martinez <thierry.martinez@inria.fr>
Co-authored-by: thierry-martinez <thierry.martinez@inria.fr>
Co-authored-by: thierry-martinez <thierry.martinez@inria.fr>
Co-authored-by: thierry-martinez <thierry.martinez@inria.fr>
@mgarnier59
Copy link
Contributor

mgarnier59 commented Feb 17, 2026

Thanks @tibisabau for updating the PR. It looks good! However I still see previous history in your PR. Do you know why that is? If you can't fix it, it will disappear when squashing and merging anyways.

Graphix requires 2 approvals before merging so another colleague will take a look soon. Thanks again for conributing!

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.

Make the noisy density matrix backend tests deterministic with the BranchSelector feature

3 participants