Define public API #1030

alecandido · 2024-09-06T14:04:59Z

Closes #790, closes #949, and closes #1031

fix broken cross-references in docs

codecov · 2024-09-06T14:19:54Z

Codecov Report

Attention: Patch coverage is 76.55678% with 64 lines in your changes missing coverage. Please review.

Project coverage is 52.34%. Comparing base (d07eb62) to head (26df7f2).
Report is 39 commits behind head on 0.2.

Files with missing lines	Patch %	Lines
src/qibolab/_core/instruments/qm/controller.py	0.00%	9 Missing ⚠️
src/qibolab/_core/instruments/qm/__init__.py	0.00%	6 Missing ⚠️
src/qibolab/_core/instruments/zhinst/executor.py	0.00%	6 Missing ⚠️
...ibolab/_core/instruments/qm/components/__init__.py	0.00%	4 Missing ⚠️
src/qibolab/instruments/era.py	0.00%	4 Missing ⚠️
src/qibolab/instruments/qm.py	0.00%	4 Missing ⚠️
src/qibolab/instruments/rohde_schwarz.py	0.00%	4 Missing ⚠️
src/qibolab/_core/instruments/qm/config/config.py	0.00%	3 Missing ⚠️
.../qibolab/_core/instruments/qm/program/arguments.py	0.00%	3 Missing ⚠️
...bolab/_core/instruments/qm/program/instructions.py	0.00%	3 Missing ⚠️
... and 12 more

Additional details and impacted files

@@            Coverage Diff             @@
##              0.2    #1030      +/-   ##
==========================================
+ Coverage   51.52%   52.34%   +0.81%     
==========================================
  Files          56       63       +7     
  Lines        2746     2820      +74     
==========================================
+ Hits         1415     1476      +61     
- Misses       1331     1344      +13

Flag	Coverage Δ
unittests	`52.34% <76.55%> (+0.81%)`	⬆️

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.

src/qibolab/__init__.py

alecandido · 2024-09-16T14:18:53Z

I decided to address also #1031 now (and not later on), because in this way we'll be able to enforce the usage of the public API.

Indeed, with the presence of _core, it won't be possible to import something private accidentally, since it will explicitly contain ._core in the qualifier.

It is taking a little bit more, but it should be the last step for 0.2 (but the README), and I deemed it relevant, since the focus we put on stability.

I'm sorry for the huge diff, but it will mostly consist of renamed files and fixed imports, including in tests and docs (which are relevant, since they will showcase the usage of the API).

As no longer necessary

In C API

To enforce the public API usage

And its downstream, i.e. zhinst

Whenever possible...

Since they do not require extra deps

Keeping the `._core` path, for the time being

alecandido · 2024-09-17T15:51:11Z

@stavros11 now, many cross-references have ._core in their qualified path. This is suboptimal, since we're documenting internal implementation details.
But that is what it was, and I didn't aim to rewrite the docs in this paper. At least, it's a clear label of something that should be reassessed...

This is a breaking change for Qibocal, postponed until 0.2 for this reason

#949 (comment)

alecandido · 2024-09-17T16:29:21Z

@stavros11 a08aaa4 is completing the implementation of #949, but it's also affecting mostly QM. If you can, during your review, confirm that QM is still working as expected.

src/qibolab/_core/__init__.py

stavros11

Thanks @alecandido. I wrote a few more comments, but I think the only main issue to be addressed is the one regarding QubitId and QubitPairId. This will affect how I will update the qibolab0.2 branch of qibocal, so that I can also test if QM is working with the usual routines (so it is kind of blocking).

doc/source/tutorials/circuits.rst

doc/source/tutorials/lab.rst

src/qibolab/_core/dummy/platform.py

src/qibolab/_core/execution_parameters.py

tests/conftest.py

stavros11 · 2024-09-18T09:01:59Z

tests/dummy_qrc/qm_octave/platform.py

+from qibolab._core.channel import Channel, ChannelMap
+from qibolab._core.instruments.dummy import DummyLocalOscillator as LocalOscillator
+from qibolab._core.instruments.qm import Octave, OPXplus, QMController
+from qibolab._core.platform import Platform


These platforms under dummy_qrc could also serve as template on how to write platforms, so I would prefer to stick to public objects. However, these particular instances are quite outdated and given that we postponed the test refactoring to a later release, instead of doing the effort to update them, I would suggest to just remove these files.

I believe that the tests using them are skipped anyway.

Yes, I applied the same policy of the docs: do not take a decision, apply the minimal update.

But your suggestion makes more sense, and I agree about dropping this (reintroducing at a later stage as a proper example).

stavros11 · 2024-09-18T09:02:20Z

tests/dummy_qrc/zurich/platform.py

    ZiAcquisitionConfig,
    ZiChannel,
    ZiDcConfig,
    ZiIqConfig,
    Zurich,
 )
-from qibolab.kernels import Kernels
-from qibolab.parameters import Parameters
+from qibolab._core.kernels import Kernels


Following the previous comment, I don't think this path even exists anymore.

No, I just completely ignored Zurich. This was the result of a bulk change, that I just post-controlled to not break anything relevant. I did not check it made sense in all the affected files.

I will apply the same resolution above, and remove the file.

stavros11 · 2024-09-18T13:03:16Z

About docs, I just tried to compile locally and I see that the API reference still has some references to internal packages, eg. "qibolab.compilers package", "qibolab.pulses package", etc and related submodules. The pages are empty, but still there. I have not checked but there are probably some leftovers within the sphinx code (.rst files). We should probably remove these.

alecandido · 2024-09-18T13:21:34Z

About docs, I just tried to compile locally and I see that the API reference still has some references to internal packages, eg. "qibolab.compilers package", "qibolab.pulses package", etc and related submodules. The pages are empty, but still there. I have not checked but there are probably some leftovers within the sphinx code (.rst files). We should probably remove these.

Have you tried removing all the gitignored content in your repo?

I.e.:

git clean -fdx doc/

stavros11 · 2024-09-18T14:08:12Z

Have you tried removing all the gitignored content in your repo?

I.e.:
git clean -fdx doc/

That worked, thanks.

stavros11

@stavros11 a08aaa4 is completing the implementation of #949, but it's also affecting mostly QM. If you can, during your review, confirm that QM is still working as expected.

I just updated qibocal and the platform and tested the single shot, Rabi amplitude and length routines and they all work on hardware. I redefined QubitId and QubitPairId in qibocal.auto.operation for that purpose (qiboteam/qibocal@a905fb4).

At these stage I believe the only open discussions are:

ExecutionParameters (could even be done in a different PR),
expose PLATFORMS (environment variable name) to public API or equivalent (trivial),
remove dummy_qrc platforms from tests (not fully relevant since tests are not strictly part of the qibolab package, thus internal). Btw, on this, I am already updating the QM platform for tests in Tests for QUA programs #1039, so you could even leave as it is here.

alecandido · 2024-09-18T15:46:13Z

remove dummy_qrc platforms from tests (not fully relevant since tests are not strictly part of the qibolab package, thus internal). Btw, on this, I am already updating the QM platform for tests in

Ok, this I will as it is (including Zurich), but I'll address the others, including the docs, but excluding ExecutionParameters (at least if we decide it's too late, it's not affecting the rest).

alecandido · 2024-09-18T17:12:20Z

I'm still missing a test for locate_platform(). I'll provide it asap.

alecandido · 2024-09-19T11:46:36Z

@stavros11 this is now ready, I'll merge as soon as the CI will pass

alecandido added this to the Qibolab 0.2.0 milestone Sep 6, 2024

This was linked to issues Sep 6, 2024

Public API #790

Closed

Check derivative imports #949

Closed

stavros11 mentioned this pull request Sep 8, 2024

Review docs for 0.2 #1028

Merged

5 tasks

stavros11 reviewed Sep 9, 2024

View reviewed changes

src/qibolab/__init__.py Outdated Show resolved Hide resolved

alecandido force-pushed the public-api branch from 95eab21 to 6c74127 Compare September 14, 2024 12:40

alecandido linked an issue Sep 16, 2024 that may be closed by this pull request

Move content to a _core subpackage #1031

Closed

alecandido added 21 commits September 16, 2024 16:35

refactor: Start defining the public api through all module attribute

98bd880

build: Add ruff and pycln configs, to treat init files consistently

934e161

refactor: Drop pycln directive

9be1708

As no longer necessary

test: Import qasm executor from defining module

54bdc74

docs: Import qasm executor from defining module

2034813

fix: Import qasm executor from defining module

ad96503

In C API

refactor: Add pulses and sequence exports

6896f75

feat!: Expose sweeeper API

ae2442c

feat!: Expose components API

2381c0e

fix: Avoid exposing configs

a18449f

fix: Move qibolab content to private core subpackage

3c26826

To enforce the public API usage

fix: Correct imports to adapt to core subpackage

434088f

fix: Fix qm imports of other qibolab modules

9af9fd3

fix: Add instruments public subpackage

aa87254

feat: Expose qm drivers

db71cc5

feat: Expose era drivers

8d511af

feat: Expose bluefors drivers

2885b6b

feat: Expose rohde & schwarz drivers

a087997

feat: Expose drivers for the dummy instruments

40186d6

fix: Fix import in C API

8b594ca

fix: Remove unneeded scoped import

f22c164

alecandido added 8 commits September 16, 2024 17:36

test: Fix imports in conf.py

5894135

And its downstream, i.e. zhinst

build: Update pylint whitelist with new qualified path

3c036f9

feat: Expose dummy platform creation

d488702

docs: Redirect imports to use public api

9482e4a

Whenever possible...

feat: Expose config kinds and qubits

f401521

feat: Expose dummy and bluefors directly in instruments

cbe8c77

Since they do not require extra deps

feat: Expose dummy and bluefors directly in instruments

87277cc

Since they do not require extra deps

docs: Fix some references

76c37e2

Keeping the `._core` path, for the time being

alecandido requested a review from stavros11 September 17, 2024 15:44

alecandido marked this pull request as ready for review September 17, 2024 15:44

alecandido added 2 commits September 17, 2024 18:24

fix: Remove unused import

a16d3cb

This is a breaking change for Qibocal, postponed until 0.2 for this reason

refactor: Standardize import, imposing mild conventions

a08aaa4

#949 (comment)

stavros11 reviewed Sep 18, 2024

View reviewed changes

src/qibolab/_core/__init__.py Show resolved Hide resolved

stavros11 reviewed Sep 18, 2024

View reviewed changes

alecandido mentioned this pull request Sep 6, 2024

Move content to a _core subpackage #1031

Closed

stavros11 approved these changes Sep 18, 2024

View reviewed changes

docs: Remove mentions to the internal API in the lab tutorial

8daa663

alecandido mentioned this pull request Sep 18, 2024

Replace ExecutionParameters in the public API #1047

Closed

feat: Expose the platform locating function

9a889b4

test: Add test for platform localization

26df7f2

alecandido merged commit dff7479 into 0.2 Sep 19, 2024
28 checks passed

alecandido deleted the public-api branch September 19, 2024 13:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Define public API #1030

Define public API #1030

alecandido commented Sep 6, 2024 •

edited

Loading

codecov bot commented Sep 6, 2024 •

edited

Loading

alecandido commented Sep 16, 2024

alecandido commented Sep 17, 2024

alecandido commented Sep 17, 2024 •

edited

Loading

stavros11 left a comment

stavros11 Sep 18, 2024

alecandido Sep 18, 2024

stavros11 Sep 18, 2024

alecandido Sep 18, 2024

stavros11 commented Sep 18, 2024

alecandido commented Sep 18, 2024

stavros11 commented Sep 18, 2024

stavros11 left a comment •

edited

Loading

alecandido commented Sep 18, 2024

alecandido commented Sep 18, 2024

alecandido commented Sep 19, 2024 •

edited

Loading

Define public API #1030

Define public API #1030

Conversation

alecandido commented Sep 6, 2024 • edited Loading

codecov bot commented Sep 6, 2024 • edited Loading

Codecov Report

alecandido commented Sep 16, 2024

alecandido commented Sep 17, 2024

alecandido commented Sep 17, 2024 • edited Loading

stavros11 left a comment

Choose a reason for hiding this comment

stavros11 Sep 18, 2024

Choose a reason for hiding this comment

alecandido Sep 18, 2024

Choose a reason for hiding this comment

stavros11 Sep 18, 2024

Choose a reason for hiding this comment

alecandido Sep 18, 2024

Choose a reason for hiding this comment

stavros11 commented Sep 18, 2024

alecandido commented Sep 18, 2024

stavros11 commented Sep 18, 2024

stavros11 left a comment • edited Loading

Choose a reason for hiding this comment

alecandido commented Sep 18, 2024

alecandido commented Sep 18, 2024

alecandido commented Sep 19, 2024 • edited Loading

alecandido commented Sep 6, 2024 •

edited

Loading

codecov bot commented Sep 6, 2024 •

edited

Loading

alecandido commented Sep 17, 2024 •

edited

Loading

stavros11 left a comment •

edited

Loading

alecandido commented Sep 19, 2024 •

edited

Loading