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

Let the pulse coeff handle complex coefficients #149

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Wikstahl
Copy link

This fixes issue #148

@BoxiLi
Copy link
Member

BoxiLi commented Jun 18, 2022

Hi @Wikstahl, thanks a lot for the PR!

It is probably better to set the dtype of the new coefficient array to be the same as the old one, instead of always using a complex array.

Could you also add a new test for the complex coefficients?

@Wikstahl
Copy link
Author

Wikstahl commented Aug 3, 2022

Sure I can fix this! :)

@BoxiLi
Copy link
Member

BoxiLi commented Mar 14, 2023

I just pushed a patch with dtype=old_coeffs.dtype

This should make the test pass. However, it does not mean that the complex coefficient is fully supported. The Processor.plot_pulse will still get a warning because of the complex numbers. But this at least lets the computation run without an error.

In fact, one can always rewrite complex coefficients by real ones with two sx and sy control terms. So real coefficient also has no loss of generality.

@BoxiLi BoxiLi changed the title Fixes #148 Let the pulse coeff handle complex coefficients Mar 14, 2023
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.

2 participants