qiboteam / qiboml

Quantum Machine Learning using Qibo
Apache License 2.0
8 stars 2 forks source link

Moving expval calculation into backends and adding torch differentiation #26

Closed MatteoRobbiati closed 3 months ago

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 0% with 47 lines in your changes missing coverage. Please review.

Project coverage is 14.23%. Comparing base (401f603) to head (5f9d75b).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/qiboteam/qiboml/pull/26/graphs/tree.svg?width=650&height=150&src=pr&token=9HMBD3N3D6&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam)](https://app.codecov.io/gh/qiboteam/qiboml/pull/26?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) ```diff @@ Coverage Diff @@ ## sym #26 +/- ## ========================================== - Coverage 15.66% 14.23% -1.44% ========================================== Files 8 8 Lines 466 513 +47 ========================================== Hits 73 73 - Misses 393 440 +47 ``` | [Flag](https://app.codecov.io/gh/qiboteam/qiboml/pull/26/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/qiboteam/qiboml/pull/26/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | `14.23% <0.00%> (-1.44%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/qiboteam/qiboml/pull/26?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | Coverage Δ | | |---|---|---| | [src/qiboml/backends/tensorflow.py](https://app.codecov.io/gh/qiboteam/qiboml/pull/26?src=pr&el=tree&filepath=src%2Fqiboml%2Fbackends%2Ftensorflow.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam#diff-c3JjL3FpYm9tbC9iYWNrZW5kcy90ZW5zb3JmbG93LnB5) | `0.00% <0.00%> (ø)` | | | [src/qiboml/backends/pytorch.py](https://app.codecov.io/gh/qiboteam/qiboml/pull/26?src=pr&el=tree&filepath=src%2Fqiboml%2Fbackends%2Fpytorch.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam#diff-c3JjL3FpYm9tbC9iYWNrZW5kcy9weXRvcmNoLnB5) | `0.00% <0.00%> (ø)` | |
alecandido commented 3 months ago

Why does Qiboml need a further function in the backend?

This is coupling once more the frontends with the backends, and defeating the original purpose of making them independent. So, to have a circuit on hardware (QibolabBackend) differentiable by Pytorch, we will need to maintain a whole PytorchBackend, same for each possible other library, that in principle could be extended even beyond TensorFlow + Pytorch + JAX at the price of implementing a single function, not a whole backend.

Execution and derivative presentation are completely separate domain, thus we should keep them separate to preserve modularity.

MatteoRobbiati commented 3 months ago

Why does Qiboml need a further function in the backend?

This is coupling once more the frontends with the backends, and defeating the original purpose of making them independent. So, to have a circuit on hardware (QibolabBackend) differentiable by Pytorch, we will need to maintain a whole PytorchBackend, same for each possible other library, that in principle could be extended even beyond TensorFlow + Pytorch + JAX at the price of implementing a single function, not a whole backend.

Execution and derivative presentation are completely separate domain, thus we should keep them separate to preserve modularity.

This is extremely true. Thanks for catching this before further modifications. While experimenting, yesterday, I lost this fundamental point. I'll move all the torch features into the sym backend (that part is working anyway) and close this.