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

Radix Broadcasting #223

Open
edyounis opened this issue Feb 27, 2024 · 8 comments
Open

Radix Broadcasting #223

edyounis opened this issue Feb 27, 2024 · 8 comments
Labels
feature request New feature or request good first issue Good for newcomers

Comments

@edyounis
Copy link
Member

edyounis commented Feb 27, 2024

In qudit-based quantum computing, the radix of each qudit determines how many classical states there are for that qudit. For example, a qubit has radix 2, and a qutrit has radix 3. BQSKit supports mixed-radix computing in many situations, which means many functions and classes accept a radixes parameter, specifying the radix of each qudit in the system being described. For example, a two-qudit gate that operates on a qubit and a qutrit would have radixes [2, 3]. Take a look at the UnitaryMatrix class __init__ for an example where some defaults are assumed.

Mixed radix is very rare. Most of the time, a single radix is used to describe every qudit in a system, i.e., qutrits, ququarts, etc. Wherever a radix is requested as a parameter/input, we should allow either a single radix or a sequence of radices. If it is clear from the context how many qudits are in the system, then a single integer radix will be broadcast for the whole system.

@edyounis edyounis added feature request New feature or request good first issue Good for newcomers labels Feb 27, 2024
@AbdullahKazi500
Copy link

Hi @edyounis can you check the #246 it addresses the radix issue

@edyounis
Copy link
Member Author

You are on the right track, but no this doesn't solve the issue. There are many places in the code where radixes are accepted as input to a function or a class, we should implement your logic everywhere applicable. Do a project wide grep for radixes to find the spots.

Also, do your changes in place, in your PR you made a new file called radix.py. I initially thought you were planning on making a Radixes class and then having some static factories to avoid repeating logic. Then you could do a RadixesLike type alias for the input parameters, that would be awesome!

I am on travel right now, so I didn't have time to check the logic rigourously. The log divided by the log is correct, but at a first glance I am a bit worried the rounding might go in the wrong direction. It would be good to add some tests for this.

Thanks for taking this issue, would you like me to assign you to it properly?

@AbdullahKazi500
Copy link

Yes please assign it to me @edyounis

@edyounis
Copy link
Member Author

edyounis commented Jun 1, 2024

Hey I realized I only should have assigned this once completed.

@AbdullahKazi500
Copy link

This modification ensures that the constructor for UnitaryMatrix accepts either a single radix or a sequence of radices for the radixes parameter and applies the appropriate logic for calculating radixes if not explicitly provided

@AbdullahKazi500
Copy link

AbdullahKazi500 commented Jun 1, 2024

incorporated the Radixes class as suggested. defined the Radixes class to handle the logic related to radixes, including inferring radixes from the dimension of the input matrix. also created a RadixesLike type alias for the input parameters to ensure consistency added unit tests
@edyounis Hi ed can you let me know if this looks good if it LGTY then i will add a few more tests

@edyounis
Copy link
Member Author

edyounis commented Jun 7, 2024

Your PRs do not address this issue or my comments. I have closed them.

@AbdullahKazi500
Copy link

@edyounis Hi Ed it was great working on this issue maybe next time thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants