qiboteam / qibolab

Quantum hardware module and drivers for Qibo.
https://qibo.science
Apache License 2.0
43 stars 16 forks source link

Add properties in QibolabBackend #1076

Closed csookim closed 1 month ago

csookim commented 1 month ago

Qibo needs qubits, connectivity, and natives to create a default transpiler.

https://github.com/qiboteam/qibo/blob/65bdfee7f5d30eb1b18cb31e54a28bac5bfd2e1a/src/qibo/backends/abstract.py#L32-L50

For native_gates, I returned all the native gates that can be executed on at least one case... (couplers are excluded) Please let me know if there is a better approach.

Checklist:

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 69.89%. Comparing base (a473d37) to head (a30c552). Report is 18 commits behind head on 0.1.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## 0.1 #1076 +/- ## ========================================== + Coverage 69.82% 69.89% +0.07% ========================================== Files 64 64 Lines 6859 6876 +17 ========================================== + Hits 4789 4806 +17 Misses 2070 2070 ``` | [Flag](https://app.codecov.io/gh/qiboteam/qibolab/pull/1076/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/qibolab/pull/1076/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | `69.89% <100.00%> (+0.07%)` | :arrow_up: | 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.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

alecandido commented 1 month ago

I rebased once more on top of #1067, to avoid the corresponding workflow to keep failing.

In order to pull, now use git pull --rebase.

alecandido commented 1 month ago

Ok, in the meanwhile, #1067 got merged (by me :P). Thus, this PR is pointing again to https://github.com/qiboteam/qibolab/tree/0.1 (GitHub updated it automatically), but the updated one :)

Still, pull as described above.

alecandido commented 1 month ago

@csookim asdict() was a link because the function is not a built-in. You should import that.

csookim commented 1 month ago

@csookim asdict() was a link because the function is not a built-in. You should import that.

Yes, but there seems to be a bug in asdict(). I also get a RecursionError. I think it would be better to replace it using fields. I will request another review after changing it.

https://github.com/python/cpython/issues/94345

https://github.com/sqlalchemy/sqlalchemy/issues/9785

src/qibolab/backends.py:67: in natives
    native_gates |= {k for k, v in asdict(q.native_gates).items() if v is not None}
../../../.pyenv/versions/3.11.10/lib/python3.11/dataclasses.py:1286: in asdict
    return _asdict_inner(obj, dict_factory)
../../../.pyenv/versions/3.11.10/lib/python3.11/dataclasses.py:1293: in _asdict_inner
    value = _asdict_inner(getattr(obj, f.name), dict_factory)
../../../.pyenv/versions/3.11.10/lib/python3.11/dataclasses.py:1293: in _asdict_inner
    value = _asdict_inner(getattr(obj, f.name), dict_factory)
../../../.pyenv/versions/3.11.10/lib/python3.11/dataclasses.py:1293: in _asdict_inner
    value = _asdict_inner(getattr(obj, f.name), dict_factory)
../../../.pyenv/versions/3.11.10/lib/python3.11/dataclasses.py:1293: in _asdict_inner
    value = _asdict_inner(getattr(obj, f.name), dict_factory)
E   RecursionError: maximum recursion depth exceeded while calling a Python object

Also, this part generates an error in pre-commit. Should I open an issue?

  - repo: https://github.com/PyCQA/docformatter
    rev: v1.7.5
    hooks:
      - id: docformatter
        additional_dependencies: [tomli]
        args: [--in-place, --config, ./pyproject.toml]
alecandido commented 1 month ago

Also, this part generates an error in pre-commit. Should I open an issue?

pre-commit v4 has been a breaking change, and that's why the CI is failing as well...

The issue is already fixed in docformatter's master branch, but that's not yet released... I'm waiting for https://github.com/PyCQA/docformatter/issues/293, before manually disabling docformatter in all our projects (otherwise I will have to do that, and manually re-enable when the release will be out again).

alecandido commented 1 month ago

Yes, but there seems to be a bug in asdict(). I also get a RecursionError. I think it would be better to replace it using fields. I will request another review after changing it.

Ah, yes - dataclasses have limited serialization capabilities (that's why we transitioned to Pydantic in 0.2). Though I expected that one to be ok, but apparently it is not. fields() should be alright, I'm re-reviewing.

alecandido commented 1 month ago

@csookim I rebased the PR on 0.1, after merging #1080. You may need to pull with git pull --rebase (if you are not already using it by default).

I will do the same for #1079, which is on top of this one.