Add support for multiple platform paths

[alecandido]

This is mainly an extension of the platform loading feature that I had in mind since long (i.e. just treat $QIBOLAB_PLATFORMS as most of the other $PATH-like variables, with multiple paths and fallback).

I started implementing this as a service for #849, to have both dummy_qrc and emulators in scope. However, I doubt it will be useful there, since I will just implement a separate fixture for that (though I could convert dummy_qrc to a more generic platforms fixture, up for voting).

In the process, I started narrowing the exports of Qibolab, since I was already touching __init__.py. More or less going in the direction of #790.

Ok, this is usually not a great idea, and I'm already kind of regretting...
I hope that is not breaking anything (things not listed in __all__ are exported by accident, and should not be used), but I might split it anyhow...

[alecandido]

@Jacfomg, since you added them: why are these exported top-level?

qibolab/src/qibolab/__init__.py

Lines 9 to 13 in b93ee1b

	from qibolab.execution_parameters import (
	AcquisitionType,
	AveragingMode,
	ExecutionParameters,
	)

We could export even more, but currently we are not doing it, so it's rather asymmetric (e.g. Pulse and PulseSequence are not exported top-level, though they are arguably as relevant as ExecutionParameters, or even more).

@andrea-pasquale removing this would break Qibocal? In any case, we could do it in 0.2, and that would not be a big deal (wrt everything else there...)

[codecov]

Codecov Report

Attention: Patch coverage is 98.57143% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 66.90%. Comparing base (485f416) to head (4972ed9).

Files	Patch %	Lines
src/qibolab/platform/load.py	96.77%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #903      +/-   ##
==========================================
+ Coverage   66.81%   66.90%   +0.08%     
==========================================
  Files          55       58       +3     
  Lines        5970     5983      +13     
==========================================
+ Hits         3989     4003      +14     
+ Misses       1981     1980       -1     

Flag	Coverage Δ
unittests	66.90% <98.57%> (+0.08%)	⬆️

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.

[andrea-pasquale]

@andrea-pasquale removing this would break Qibocal? In any case, we could do it in 0.2, and that would not be a big deal (wrt everything else there...)

Yes, it will break qibocal but it should be an easy fix.

[alecandido]

Yes, it will break qibocal but it should be an easy fix.

At the moment, I'm not touching it.

I will do it in 0.2

[alecandido]

Despite a bunch of reshuffling, this is intended to target 0.1.x (I added the milestone to make this clear).

For this reason, we should test that it's not breaking Qibocal, nor any platform working with the current Qibolab main. I would kindly ask the reviewers (@stavros11 and @andrea-pasquale) to do this, since, for you, it should be just a matter of reinstalling Qibolab with this branch in an environment you're already using for something else.

All the things I've moved around should have been considered internal, so it is formally non-breaking (but I hope it's non-breaking even in practice).

[Jacfomg]

@Jacfomg, since you added them: why are these exported top-level?

I don't remenber who, but it was for the qibocal imports. Yes, changing would requiere changing it the import on each routine but only that.

[stavros11]

For this reason, we should test that it's not breaking Qibocal, nor any platform working with the current Qibolab main. I would kindly ask the reviewers (@stavros11 and @andrea-pasquale) to do this, since, for you, it should be just a matter of reinstalling Qibolab with this branch in an environment you're already using for something else.

For now I get an error coming from this line in qibocal: https://github.com/qiboteam/qibocal/blob/37d060599c739b3b70602505d29509af295aedb0/src/qibocal/protocols/randomized_benchmarking/utils.py#L9
This is expected given the changes in this PR, however it is clearly a qibocal issue: why is defaultdict imported from qibolab instead of collections (standard library)?

Also, unrelated with the above point, but I see that tests here are failing for windows. Maybe some path related convention?

EDIT: Apart from the defaultdict issue, qibocal seems to work with this branch, at least the single shot and allXY that I tried (but is unlikely that some other routine is broken, given the changes).

[Jacfomg]

For this reason, we should test that it's not breaking Qibocal, nor any platform working with the current Qibolab main. I would kindly ask the reviewers (@stavros11 and @andrea-pasquale) to do this, since, for you, it should be just a matter of reinstalling Qibolab with this branch in an environment you're already using for something else.

For now I get an error coming from this line in qibocal: https://github.com/qiboteam/qibocal/blob/37d060599c739b3b70602505d29509af295aedb0/src/qibocal/protocols/randomized_benchmarking/utils.py#L9 This is expected given the changes in this PR, however it is clearly a qibocal issue: why is defaultdict imported from qibolab instead of collections (standard library)?

Also, unrelated with the above point, but I see that tests here are failing for windows. Maybe some path related convention?

EDIT: Apart from the defaultdict issue, qibocal seems to work with this branch, at least the single shot and allXY that I tried (but is unlikely that some other routine is broken, given the changes).

Yes the import should not be like that, I will fix it.

[alecandido]

Ok, great!

I just have to fix a test (because I need the path to be escaped for Windows), and on my side this is ready.

@stavros11 in case you're interested in, the problem is that the path separator on Windows is the same as the escape character, i.e. \. Thus, the path has to be escaped, but only on Windows...
(in any case, it is only a problem with the tests, Qibolab is working perfectly fine with both, if the path is suitably specified according to the platform)

[stavros11]

Thanks @alecandido, looks good to me. My only concern is that the file reshuffling may cause some weird conflicts with 0.2, but if you are planning to do the rebase I don't have any objection.

[stavros11]

This is probably trying to reproduce the previous behavior, however in principle we don't need to necessarily expose unroll_sequences externally. I think it is only used by the Platform internally.

[alecandido]

Point is that I don't know what is used and what not, so I kept it to avoid breaking anything.

But what you say makes sense to me, so we could even immediately drop from 0.2.

[alecandido]

but if you are planning to do the rebase I don't have any objection.

I have to take care of my own mess :P
I wanted to move even backends.py under platforms/ (before the backend and the platform have a very similar role, essentially the backend is just an interface for the platform), but I saved that for 0.2.

[andrea-pasquale]

Thanks @alecandido, looks good to me.
I just have a suggestion.

[alecandido]

@stavros11 @andrea-pasquale: I've just rebased, so, as soon as the tests will pass, and given the double approval, I'll directly merge.