Skip to content
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

Temporary fix of error mitigation routine if QIBO_PLATFORM is not set #1063

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stavros11
Copy link
Member

The error mitigation routine relies on the GlobalBackend in a few places. This causes issues when the QIBO_PLATFORM environment variable is not properly set. Usually we do not see these issues because this environment variable is set in the slurm prologue, however this is not the case for the qw5q_platinum node.

Copy link

codecov bot commented Jan 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.35%. Comparing base (293e2a9) to head (53a94ca).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1063      +/-   ##
==========================================
+ Coverage   97.32%   97.35%   +0.03%     
==========================================
  Files         124      124              
  Lines        9893     9894       +1     
==========================================
+ Hits         9628     9632       +4     
+ Misses        265      262       -3     
Flag Coverage Δ
unittests 97.35% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/qibocal/protocols/readout_mitigation_matrix.py 100.00% <100.00%> (+2.50%) ⬆️

... and 2 files with indirect coverage changes

Comment on lines +65 to +66
os.environ["QIBO_PLATFORM"] = platform.name
backend = QibolabBackend(platform)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait: if you directly use QibolabBackend [*], then $QIBO_PLAFORM should be irrelevant. Isn't it?

If that's the case, this should be sufficient

Suggested change
os.environ["QIBO_PLATFORM"] = platform.name
backend = QibolabBackend(platform)
backend = QibolabBackend(platform)

[*]: which is a great idea - until we'll reintroduce support in Qibocal for backends other than Qibolab #913 (but that's a separate issue, to tackle later on)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait: if you directly use QibolabBackend [*], then $QIBO_PLAFORM should be irrelevant. Isn't it?

That is also what I initially expected, however it seems that another instance of GlobalBackend is instantiated within the

_, results = execute_transpiled_circuit(

call which causes a failure if the environment variable is not properly set (which is the case if the slurm prologue is wrong). From the error trace, it seemed that this second GlobalBackend is related to the transpilation process, but I did not investigate in detail.

In any case, this PR was just a quick hotfix for something that @Luca-Ben-Herrmann was running, caused by the wrong slurm prologue of qw5q_platinum partition (which has also been fixed). Ideally, I would like to make all routines independent of the GlobalBackend to avoid such issues, but I am not sure how much of rework that requires.

[*]: which is a great idea - until we'll reintroduce support in Qibocal for backends other than Qibolab #913 (but that's a separate issue, to tackle later on)

I guess hard-coding QibolabBackend here is also not optimal as it would not work for simulation, which I am not sure if is relevant for that particular routine. Anyway, this PR is mostly to raise these issues, maybe we don't even need to merge as it is, unless you are all fine with restricting with qibolab for now and introducing other backends later, as you are proposing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess hard-coding QibolabBackend here is also not optimal as it would not work for simulation,

As said, I would not worry too much about this. Not yet.

call which causes a failure if the environment variable is not properly set (which is the case if the slurm prologue is wrong). From the error trace, it seemed that this second GlobalBackend is related to the transpilation process, but I did not investigate in detail.

Actually, I failed to find it, since in that call the backend you're instantiating is passed in (and transitively passed down into the called transpile_circuits() function as well).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants