Skip to content

Update PreSamplex.add_combine_node() to avoid adding the trivial combination#311

Merged
joshuasn merged 9 commits intomainfrom
remove-useless-combine
Mar 10, 2026
Merged

Update PreSamplex.add_combine_node() to avoid adding the trivial combination#311
joshuasn merged 9 commits intomainfrom
remove-useless-combine

Conversation

@ihincks
Copy link
Copy Markdown
Collaborator

@ihincks ihincks commented Feb 23, 2026 •
edited
Loading

Summary

Details and comments

AI tool use: claude opus 4.6

This branch timing:

image

main:
image

ihincks
ihincks commented Feb 23, 2026
View reviewed changes
Comment thread samplomatic/samplex/nodes/inject_noise_node.py Outdated

import numpy as np
from qiskit.quantum_info import PauliLindbladMap

Copy link
Copy Markdown
Collaborator Author

@ihincks ihincks Feb 23, 2026

Choose a reason for hiding this comment

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

unrelated to this PR, not sure how it got past pre-commit

joshuasn
joshuasn previously approved these changes Feb 23, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@joshuasn joshuasn left a comment

Choose a reason for hiding this comment

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

nice, lgtm

@joshuasn joshuasn dismissed their stale review February 23, 2026 21:35

didn't look at timings closely..

@ihincks
Copy link
Copy Markdown
Collaborator Author

ihincks commented Feb 23, 2026

so better, but not crazy

  ┌───────────────────────────┬───────────┬───────────┬─────────────────┐
  │          Metric           │   main    │  branch   │      delta      │
  ├───────────────────────────┼───────────┼───────────┼─────────────────┤
  │ Node counts               │           │           │                 │
  ├───────────────────────────┼───────────┼───────────┼─────────────────┤
  │ Total nodes               │ 1,565     │ 1,149     │ -416 (-27%)     │
  ├───────────────────────────┼───────────┼───────────┼─────────────────┤
  │ SliceRegisterNode         │ 626       │ 210       │ -416 (-66%)     │
  ├───────────────────────────┼───────────┼───────────┼─────────────────┤
  │ Evaluation nodes          │ 1,354     │ 938       │ -416 (-31%)     │
  ├───────────────────────────┼───────────┼───────────┼─────────────────┤
  │ Evaluation streams        │ 13        │ 9         │ -4 (-31%)       │
  ├───────────────────────────┼───────────┼───────────┼─────────────────┤
  │ Sampling/Collection/other │ unchanged │ unchanged │                 │
  ├───────────────────────────┼───────────┼───────────┼─────────────────┤
  │ Serialization             │           │           │                 │
  ├───────────────────────────┼───────────┼───────────┼─────────────────┤
  │ Serialized size           │ 5,288 KB  │ 5,187 KB  │ -101 KB (-1.9%) │
  ├───────────────────────────┼───────────┼───────────┼─────────────────┤
  │ Serialize time            │ 500 ms    │ 454 ms    │ -46 ms (-9%)    │
  ├───────────────────────────┼───────────┼───────────┼─────────────────┤
  │ Timings                   │           │           │                 │
  ├───────────────────────────┼───────────┼───────────┼─────────────────┤
  │ Build time                │ 4,488 ms  │ 4,556 ms  │ +68 ms (~noise) │
  ├───────────────────────────┼───────────┼───────────┼─────────────────┤
  │ Sample time (100 rand)    │ 225 ms    │ 212 ms    │ -13 ms (-6%)    │
  ├───────────────────────────┼───────────┼───────────┼─────────────────┤
  │ Per randomization         │ 2.25 ms   │ 2.12 ms   │ -0.13 ms (-6%)  │
  └───────────────────────────┴───────────┴───────────┴─────────────────┘

@joshuasn
Copy link
Copy Markdown
Collaborator

joshuasn commented Feb 25, 2026

@ihincks still a good improvement, lgtm

@ihincks ihincks marked this pull request as ready for review March 9, 2026 22:39
joshuasn
joshuasn approved these changes Mar 10, 2026
View reviewed changes
Comment thread samplomatic/pre_samplex/pre_samplex.py
)


def _match_propagate_group(
Copy link
Copy Markdown
Collaborator

@joshuasn joshuasn Mar 10, 2026

Choose a reason for hiding this comment

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

this is a great change, tysm

@joshuasn joshuasn merged commit 672bf3a into main Mar 10, 2026
8 checks passed
@joshuasn joshuasn deleted the remove-useless-combine branch March 10, 2026 13:26
@joshuasn joshuasn mentioned this pull request Mar 10, 2026
Closed
@ihincks ihincks mentioned this pull request Mar 10, 2026
Merged
ihincks added a commit that referenced this pull request Mar 10, 2026
@ihincks
Add a changelog for 311, and memoize its helper (#322)
b4a7178 
## Summary

This PR adds a changelog for #311 and memoizes the propagation helper
function.

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

Reviewers

@joshuasn joshuasn joshuasn approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ihincks @joshuasn