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

TWPA extra #962

Merged
merged 1 commit into from
Aug 7, 2024
Merged

TWPA extra #962

merged 1 commit into from
Aug 7, 2024

Conversation

alecandido
Copy link
Member

Closes #961

It was easier to do it immediately, instead of leaving a further issue open and forget.

@alecandido alecandido force-pushed the twpa-extra branch 2 times, most recently from f1faa06 to bae20e1 Compare August 7, 2024 13:50
Copy link

codecov bot commented Aug 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 42.02%. Comparing base (1e3a501) to head (2bfcb53).

Additional details and impacted files
@@           Coverage Diff           @@
##              0.2     #962   +/-   ##
=======================================
  Coverage   42.02%   42.02%           
=======================================
  Files          74       74           
  Lines        5820     5820           
=======================================
  Hits         2446     2446           
  Misses       3374     3374           
Flag Coverage Δ
unittests 42.02% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

For the time being mirroring the los one, but better to keep them independent, since the two things may split.
And it's pretty cheap anyhow...
Copy link
Member

@stavros11 stavros11 left a comment

Choose a reason for hiding this comment

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

Okay, but I still don't see how this is any useful given that if someone knows how to do pip install .[twpa] they could just as well do pip install .[los]. The missing piece is documentation.

@alecandido
Copy link
Member Author

Okay, but I still don't see how this is any useful given that if someone knows how to do pip install .[twpa] they could just as well do pip install .[los]. The missing piece is documentation.

Ah, yes, definitely.

This is not useful on its own, just "clearer". I.e., if you need a TWPA, but not an LO, then you could use pip install .[twpa], instead of pip install .[los], when you don't have LOs.

But the relevant one is certainly qiboteam/qibolab_platforms_qrc#163

@alecandido alecandido merged commit 80bc067 into 0.2 Aug 7, 2024
22 checks passed
@alecandido alecandido deleted the twpa-extra branch August 7, 2024 19:41
@stavros11
Copy link
Member

This is not useful on its own, just "clearer". I.e., if you need a TWPA, but not an LO, then you could use pip install .[twpa], instead of pip install .[los], when you don't have LOs.

Strictly speaking, qibolab is not controlling the TWPA itself but the TWPA pump, which in all cases so far it happens to actually be an LO. :) (no issues with merging this though, fortunately it is not causing any conflicts)

@alecandido
Copy link
Member Author

alecandido commented Aug 7, 2024

Strictly speaking, qibolab is not controlling the TWPA itself but the TWPA pump, which in all cases so far it happens to actually be an LO. :) (no issues with merging this though, fortunately it is not causing any conflicts)

Ok, this is formally correct, but slightly less intuitive.

To be fair, both los and twpa are a bit out of the usual scheme. Indeed, we don't have extras for controller, so we may separate by vendor, and possibly specific product.

However, we are not really at the level of being that strict on this, and even los alone is not that bad. It was just confusing me.
But, if this is worse, we could even revert.

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