-
Notifications
You must be signed in to change notification settings - Fork 4
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
Classify qubit states #261
Conversation
…nto classify-qubit-states
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #261 +/- ##
==========================================
+ Coverage 35.11% 37.84% +2.72%
==========================================
Files 30 42 +12
Lines 2355 2801 +446
==========================================
+ Hits 827 1060 +233
- Misses 1528 1741 +213
Flags with carried forward coverage won't be shown. Click here to find out more.
|
for more information, see https://pre-commit.ci
…nto classify-qubit-states
for more information, see https://pre-commit.ci
…nto classify-qubit-states
for more information, see https://pre-commit.ci
…nto classify-qubit-states
for more information, see https://pre-commit.ci
@Edoardo-Pedicillo @scarrazza from what I understood "Ramiro"'s method is a linear SVM, simply manually fitted. Is there any value in using that, instead of an actual SVM? (or just the Naive Bayes, that is orders of magnitude faster) Of course, the current implementation is definitely useful for benchmarking, and to prove the effectiveness and performances of the other methods. |
pyproject.toml
Outdated
networkx = "^3.0" | ||
pydantic = "^1.10.5" | ||
pydot = { version = "^1.4.2", optional = true } | ||
tensorflow-io-gcs-filesystem = "0.31" |
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.
Why do you need extra IO support?
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.
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.
No, the problem is in a dependency of a dependency, do not add it to your own to make it passing on Windows.
We can think about more refined solutions, in particular dropping who depends on that, or lowering the related version, but if you don't use it directly, don't declare it among your deps :)
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.
Yes, I know, obviously tensorflow has this dependecy, I checked but downgrading it doesn't solve the problem. I can try with the other dependencies, but this could be a lot time-consuming. Do you have some hints to solve it as soon as possible?
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.
It is definitely a non-trivial problem. Proper solutions are:
- TensorFlow IO releasing wheels for Windows for v0.32.0 tensorflow-io 0.32.0 release tensorflow/io#1789 (comment)
- Poetry support to resolve indirect deps for multiple platforms Platform dependent indirect dependencies python-poetry/poetry#7746 (or at least invalidate the lock file for unsupported platforms)
I'm trying to consider if there are other possible workarounds, in order to avoid a direct dependency on tensorflow-io-gcs-filesystem
, that we don't use.
for more information, see https://pre-commit.ci
Co-authored-by: Alessandro Candido <candido.ale@gmail.com>
…nto classify-qubit-states
for more information, see https://pre-commit.ci
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.
Last few comments, but there is nothing blocking :)
After these we can merge (if needed, even before).
Co-authored-by: Alessandro Candido <candido.ale@gmail.com>
Co-authored-by: Alessandro Candido <candido.ale@gmail.com>
…nto classify-qubit-states
for more information, see https://pre-commit.ci
…nto classify-qubit-states
for more information, see https://pre-commit.ci
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.
Let's try this way: we won't have tf
on Windows, but we're mostly not even using it.
@scarrazza this PR is ready to be merged. |
Very good, thanks. |
This PR introduces the folder
qibocal/extras
, where there are the classifiers benchmarks.Checklist: