Skip to content

Conversation

@maffoo
Copy link
Contributor

@maffoo maffoo commented May 31, 2022

Fixes #5418

@maffoo maffoo requested review from a team, cduck and vtomole as code owners May 31, 2022 07:54
@maffoo maffoo requested a review from MichaelBroughton May 31, 2022 07:54
@daxfohl
Copy link
Collaborator

daxfohl commented May 31, 2022

I think the repeat function is also affected.

if self.use_repetition_ids:
loop_size = abs(self.repetitions)
if not self.repetition_ids:
object.__setattr__(self, 'repetition_ids', self._default_repetition_ids())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could all these be consolidated by having default_repetition_ids return None if use_repetition_ids is false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to do the self.use_repetition_ids check in self._default_repetition_ids.

@CirqBot CirqBot added the size: S 10< lines changed <50 label May 31, 2022
@maffoo
Copy link
Contributor Author

maffoo commented May 31, 2022

@daxfohl, I can't seem to re-request review from you, but this is ready for review again after addressing your comments. PTAL

@daxfohl
Copy link
Collaborator

daxfohl commented May 31, 2022

lgtm

@maffoo
Copy link
Contributor Author

maffoo commented May 31, 2022

@daxfohl, thanks for the review! Looks like you commented instead of approving.

Copy link
Collaborator

@dabacon dabacon left a comment

Choose a reason for hiding this comment

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

LGTM

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

Labels

size: S 10< lines changed <50

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't construct huge repetition id lists for CircuitOperation when use_repetition_ids=False

5 participants