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

Soften switching device with strict conditions #724

Merged
merged 16 commits into from
Sep 18, 2024
Merged

Conversation

a-corni
Copy link
Collaborator

@a-corni a-corni commented Aug 22, 2024

Conditions on total_bottom_detuning and bottom_detuning for DMM channels, and on EOM config for channels using EOM (eom config being the same) are deleted.
They are replaced by a condition on the building of the sequence being possible, and the samples of the Sequence being unaffected by the switching of the device.
To do this, we first loop over all the possible matchings between declared channels and devices in the new device. For the one having correct type, addressing, modulations, ... we try to build the associated sequence. If the strict condition is provided and the Sequence can be successfully built, we check that the samples are the same for the two Sequences. Finally, the Sequence built with the first compatible matching.
If no matching is found to be compatible, we go through the same error detection process as before to provide a good hint on the error.
Closes #721

@a-corni a-corni self-assigned this Aug 22, 2024
@a-corni a-corni requested a review from HGSilveri August 27, 2024 08:13
Copy link
Collaborator

@HGSilveri HGSilveri left a comment

Choose a reason for hiding this comment

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

Looking good overall! The switch_device() method is getting too bulky, how would you feel about moving it into a separate file?

pulser-core/pulser/sequence/sequence.py Outdated Show resolved Hide resolved
pulser-core/pulser/sequence/sequence.py Outdated Show resolved Hide resolved
pulser-core/pulser/sequence/sequence.py Outdated Show resolved Hide resolved
pulser-core/pulser/sequence/sequence.py Outdated Show resolved Hide resolved
@HGSilveri HGSilveri added this to the v0.20 Release milestone Sep 11, 2024
@a-corni
Copy link
Collaborator Author

a-corni commented Sep 13, 2024

Looking good overall! The switch_device() method is getting too bulky, how would you feel about moving it into a separate file?

I have created a "helpers" folder, and placed this function switch_device in it. I have also moved _seq_str.py to this file since it is also a helper function. The other files does not seem to contain only a helper function, so I have decided to let it like that.

@HGSilveri
Copy link
Collaborator

I have created a "helpers" folder, and placed this function switch_device in it. I have also moved _seq_str.py to this file since it is also a helper function. The other files does not seem to contain only a helper function, so I have decided to let it like that.

This is okay with me. I think other files could potentially go there but it's not out-of-scope for this PR.

Copy link
Collaborator

@HGSilveri HGSilveri left a comment

Choose a reason for hiding this comment

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

Only one nit, nice job!

tests/test_sequence.py Outdated Show resolved Hide resolved
@a-corni
Copy link
Collaborator Author

a-corni commented Sep 18, 2024

Ready @HGSilveri 😄

@a-corni a-corni merged commit 2f5e56c into develop Sep 18, 2024
9 checks passed
@a-corni a-corni deleted the switch_device branch September 18, 2024 09:12
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.

Avoid failing to switch device prematurely
2 participants