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

Add i64 type in external gate arguments #203

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TheGupta2012
Copy link
Member

Follow up for #201

Summary of changes

  • By default, all args in external gates were considered as double which resulted in discrepancy in the function call by pyqir
  • Adds the i64 type in the external gate arguments to account for int arguments

Copy link

codecov bot commented Feb 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@Sola85
Copy link
Contributor

Sola85 commented Feb 27, 2025

Do I see correctly that this would create new function definitions depending on the type of the arguments? I.e.

rz(1) q[0];
rz(1.1) q[0];

would result in two definitions of the rz gate (when marked as external)?
I'm wondering whether this can have unintended consequences for people using the generated qir...

I think the best fix for #201 in particular would be to make pyqir allow implicit casts from int to double.

The more general problem might be that qbraid-qir currently does not know the intended types of external gate arguments and therefore the best it can do is guess... Meaning that there will always be edge-cases that are not handled properly (given the unavailablilty of "correct" argument types).

@TheGupta2012
Copy link
Member Author

TheGupta2012 commented Feb 27, 2025

You're correct @Sola85, this fix would generate different definitions of the external gates depending on the actual arguments. We have currently fixed the parameters for all gates to be FloatLiterals when they are unrolled and thus we were using the double type in the function definitions.

Another bug which I now see is that -

rz(1) q[0];
rz(1.1) q[0];

would result in an error as rz(1.1) q[0]; will refer to the cached definition with the integer argument!

I think I'm with you where pyqir should allow implicit casts from int to double. If you wanna open an issue in pyqir, go for it otherwise I can open it later today

@TheGupta2012 TheGupta2012 added pyqir Interfacing with QIR python bindings do-not-merge PR not yet ready / contains breaking changes labels Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge PR not yet ready / contains breaking changes pyqir Interfacing with QIR python bindings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants