Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reg2const arithmetic and tracking previously set ssa vars #8

Merged
merged 182 commits into from
Oct 24, 2022

Conversation

RolandMacDoland
Copy link
Contributor

@RolandMacDoland RolandMacDoland commented Sep 21, 2022

This PR addresses the addition of two functionalities:

  • reg2const arithmetic: this requires to keep track of registers set with a constant value through the SetBitsOp for potential later reuse.
  • Keep track of ssa variables: in the same line as above, there is now functionality to keep track of previously created ssa vars for later reuse.

This work also encompasses an architectural re-design of the package in three main parts:

  • A QIR parser
  • A QIR generator
  • A module that extends the pyqir SimpleModule to handle custom define quantum functions.

pytket_qir/gatesets/pyqir/pyqir.py Outdated Show resolved Hide resolved
pytket_qir/generator.py Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
def _reg2ssa_var(self, bit_reg: BitRegister, int_size: int) -> Value:
# A utility function to convert from a pytket
# BitRegister to an SSA variable via pyqir types.
reg_name = bit_reg[0].reg_name
Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree about the first; not quite sure about the second: can we assume a ClassicalExpBox has no classical wires?

if in_width > 0:
com_bits = args[:in_width]
args = args[in_width:]
regname = com_bits[0].reg_name
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you tried with latest pytket? This error was removed in CQCL/tket#589 .

Copy link
Collaborator

@cqc-alec cqc-alec left a comment

Choose a reason for hiding this comment

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

Could you please create a (low-priority) task to make SetBits and ClassicalExpBox support the trivial no-wires case? Ideally we should support this but maybe there are issues with pytket that prevent it.

@RolandMacDoland
Copy link
Contributor Author

Could you please create a (low-priority) task to make SetBits and ClassicalExpBox support the trivial no-wires case? Ideally we should support this but maybe there are issues with pytket that prevent it.

Sure. But don't they already support it ? I thought from the examples above that setting them with empty registers was valid. Or, am I getting confused ?

Thanks for the review.

@cqc-alec
Copy link
Collaborator

Could you please create a (low-priority) task to make SetBits and ClassicalExpBox support the trivial no-wires case? Ideally we should support this but maybe there are issues with pytket that prevent it.

Sure. But don't they already support it ? I thought from the examples above that setting them with empty registers was valid. Or, am I getting confused ?

E.g. e0c46da

I still don't get why we have to raise an exception here (I mean, I get it in terms of how the code works currently, but I don't see why it has to be that way).

Anyway this is low-priority and shouldn't block merging this!

@RolandMacDoland
Copy link
Contributor Author

Could you please create a (low-priority) task to make SetBits and ClassicalExpBox support the trivial no-wires case? Ideally we should support this but maybe there are issues with pytket that prevent it.

Sure. But don't they already support it ? I thought from the examples above that setting them with empty registers was valid. Or, am I getting confused ?

E.g. e0c46da

I still don't get why we have to raise an exception here (I mean, I get it in terms of how the code works currently, but I don't see why it has to be that way).

Anyway this is low-priority and shouldn't block merging this!

Tbf, I don't understand very clearly either what you are expecting. I am a little confused between the new behaviour of the ops and handling it in the context of correctness here. Maybe best to discuss that in Slack ?

@RolandMacDoland RolandMacDoland merged commit c519406 into develop Oct 24, 2022
@RolandMacDoland RolandMacDoland deleted the reg2const-arithmetic branch October 24, 2022 16:14
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.

3 participants