-
Notifications
You must be signed in to change notification settings - Fork 17
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
Parameters updater #1086
Parameters updater #1086
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1086 +/- ##
==========================================
+ Coverage 50.55% 50.84% +0.28%
==========================================
Files 63 63
Lines 2886 2903 +17
==========================================
+ Hits 1459 1476 +17
Misses 1427 1427
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Also, since this is creating a new platform.parameters = platform.parameters.update(...) right? |
Definitely. In this sense, we can make a couple of improvements regarding the UI, renaming |
Ok, I have to admit that I've been a bit lazy with tests, since I'm only using Any tests' extension is of course welcome (in case anyone feels we definitely need more). Even clear proposals are welcome (but the effort to directly implement them should be comparable in size... in any case, I'm happy to do that, if needed). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @alecandido. I think we can merge this so that I restart working on qiboteam/qibocal#996 using it.
Ok, I have to admit that I've been a bit lazy with tests, since I'm only using
dummy
to test, anddummy
has onlyint
qubits' names. However, that should be the most challenging path, so it should be good enough for the time being.
I think testing on dummy
is sufficient in this case, since this functionality is expected to behave the same on all platforms. True about string qubits, maybe an easy way to address this and other similar issues is to add a "dummy_strings"
(and make our life more miserable), or just add some qubits with string names in the current dummy.
I could just add a minimal platform with string names in the tests. But since it's not that essential, I'll save it for later. |
This is aiming to supersede #1061, providing a method and an associated syntax to update
Platform
parameters.