Skip to content

Conversation

@babacry
Copy link
Collaborator

@babacry babacry commented Aug 28, 2025

Previous version of dd allows adding new moments to the circuit in the transformer. In some use cases, we might see a lot of new moments in the transformed circuits. This PR will fix the issues.

First, it captures the circuit's structure. Second, it inserts elements based on the structural information gathered in the initial step: the design.

@github-actions github-actions bot added the size: L 250< lines changed <1000 label Aug 28, 2025
Copy link
Collaborator

@eliottrosenberg eliottrosenberg left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @babacry!

@NoureldinYosri would you be able to take a look when you get a chance? Thanks!

Copy link
Collaborator

@NoureldinYosri NoureldinYosri left a comment

Choose a reason for hiding this comment

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

first review ... please address the comments and make the CIs pass

@babacry babacry marked this pull request as ready for review October 21, 2025 02:07
@babacry babacry requested review from a team and vtomole as code owners October 21, 2025 02:07
@codecov
Copy link

codecov bot commented Oct 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.38%. Comparing base (bc6dab9) to head (c7abf9e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7609   +/-   ##
=======================================
  Coverage   99.38%   99.38%           
=======================================
  Files        1091     1091           
  Lines       97815    97896   +81     
=======================================
+ Hits        97214    97295   +81     
  Misses        601      601           

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@babacry
Copy link
Collaborator Author

babacry commented Oct 21, 2025

Thanks Nour for the review!

Could you help take a look at the Design and taking another look at the PR? Thanks! @NoureldinYosri

Copy link
Collaborator

@NoureldinYosri NoureldinYosri left a comment

Choose a reason for hiding this comment

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

overall looks good ... some naming comments

INSERTABLE = 3

def __str__(self) -> str:
match self:
Copy link
Collaborator

Choose a reason for hiding this comment

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

an enum value has a .value which can be string

class _GateLabel(Enum):
    UNKNOWN = '?'
    WALL = 'w'
    DOOR = 'd'
    INSERTABLE = 'i'
    
    def __str__(self) -> str:
        return self.value



@frozen
class _LabeledCircuit:
Copy link
Collaborator

@NoureldinYosri NoureldinYosri Oct 25, 2025

Choose a reason for hiding this comment

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

I don't like the _LabeledCircuit/_LabeledGate names ... lets call them _Grid,_CellType since we are turning the circuit into a 2 grid where each cell has one of 4 types.

return cls(circuit=circuit, gate_types=gate_types, need_to_stop=need_to_stop)

@staticmethod
def _backward_set_stopping_slots(
Copy link
Collaborator

Choose a reason for hiding this comment

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

since this is a static private method, maybe move it outside the class go/pystyle#decorators

busy_moment_range_by_qubit = _calc_busy_moment_range_of_each_qubit(orig_circuit)
labeled_circuit = _LabeledCircuit.from_circuit(orig_circuit, single_qubit_gate_moments_only)

print(f"Preprocessed input circuit repr:\n{repr}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

there are 3 options here

  1. remove this print line
  2. hide it behind a if verbose: and set verbose to default to False
  3. turn it into a logging.info call

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

Labels

size: L 250< lines changed <1000

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants