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

allowing more than a QM existing at same time #760

Conversation

jjmartinezQT
Copy link
Collaborator

Setting the flag to allow multiple QM virtual machines at once. I think the qm.close is already executed when turning off the instrument, so if experimentalist do platform.disconnect() after each experiment, the virtual machine will be wipped off and not left floating around.

Copy link

linear bot commented Jul 24, 2024

@jjmartinezQT jjmartinezQT requested a review from fedonman July 24, 2024 09:02
Copy link

codecov bot commented Jul 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.81%. Comparing base (b037fca) to head (f5664cf).
Report is 42 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #760   +/-   ##
=======================================
  Coverage   95.81%   95.81%           
=======================================
  Files         267      267           
  Lines        8679     8683    +4     
=======================================
+ Hits         8316     8320    +4     
  Misses        363      363           
Flag Coverage Δ
unittests 95.81% <100.00%> (+<0.01%) ⬆️

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.


🚨 Try these New Features:

@jjmartinezQT jjmartinezQT requested a review from jordivallsq July 24, 2024 09:09
Copy link
Contributor

@jordivallsq jordivallsq left a comment

Choose a reason for hiding this comment

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

Is this the only requirement from QM needed to connect multiple experiments? are there some restrictions in terms of no overlaping input channels or some sort of condition that can couse a crash?

@jjmartinezQT
Copy link
Collaborator Author

jjmartinezQT commented Jul 24, 2024

Is this the only requirement from QM needed to connect multiple experiments? are there some restrictions in terms of no overlaping input channels or some sort of condition that can couse a crash?

As reported by Yifei the way to do it is to set this flag to True and ensure qm.close() is done every time (see description of this PR for when that happens in Qililab), I assume to not have zombie virtual machines all around or not to reach a limit. The overlapping of input channels I believe it should be a matter of configuration (runcard) but we need to test it with the experimentalists.

Copy link
Collaborator

@fedonman fedonman left a comment

Choose a reason for hiding this comment

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

This change could lead to a hanging QM that is not closed.

Imagine the scenario that you execute a QProgram, the QM is opened, but something goes wrong in the QUA execution in hardware. For example, the user uses a loop with a very large arbitrary array and memory overflow happens (Jordi had this issue at some point). The python execution is halted and qm.close() is never called. With close_other_machines=True this was not an issue for the next QProgram execution. With close_other_machines=False there will be a port conflict when trying to open the second QM.

I believe we should add some kind of try/finally to close the QM in any case.

@jjmartinezQT
Copy link
Collaborator Author

This change could lead to a hanging QM that is not closed.

Imagine the scenario that you execute a QProgram, the QM is opened, but something goes wrong in the QUA execution in hardware. For example, the user uses a loop with a very large arbitrary array and memory overflow happens (Jordi had this issue at some point). The python execution is halted and qm.close() is never called. With close_other_machines=True this was not an issue for the next QProgram execution. With close_other_machines=False there will be a port conflict when trying to open the second QM.

I believe we should add some kind of try/finally to close the QM in any case.

Makes sense, where do you suggest to try close the QM, execute_qprogram method?

@jjmartinezQT jjmartinezQT requested a review from fedonman August 1, 2024 10:38
Copy link
Contributor

@jordivallsq jordivallsq left a comment

Choose a reason for hiding this comment

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

I agree with the exception introduced to close only the user's connection (although the name turn off is scary for the qm.close ^^

Copy link
Collaborator

@fedonman fedonman left a comment

Choose a reason for hiding this comment

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

Seems ok for me! 💯 Have we test it in hardware with some on purpose exceptions?

Also, this might be changed in the future implementation of Session, since it will be responsible for handling these errors.

@jjmartinezQT
Copy link
Collaborator Author

jjmartinezQT commented Aug 9, 2024

Seems ok for me! 💯 Have we test it in hardware with some on purpose exceptions?

Also, this might be changed in the future implementation of Session, since it will be responsible for handling these errors.

ps: to clarify, it has been used by experimentalists over and over this branch, even when it crashes they can continue working without issue, so I will take that as hardware test.

Not yet tested in hardware, looking for the way to test it with the minimum impact to the guys measuring. Need to generate the on purpose exceptions, or a meaningful way to test this on hardware.

actions-user and others added 5 commits August 16, 2024 15:46
…778)

* add parameter class and one qubit analog transpiler

* add unit tests

* add unit tests

* move fluqe parameter inside analog, fix pylint complains

* draft of anneal program with transpiler for 1q

* add analog program execution workflow

* add changelog

* fix mypy

* fix docstring

* add unit test for anneal program

* wip platform unit tests

* add to_dict method to qprogram in order to tests that a qprogram is equivalent to another

* fix tests

* fix pylint

* fix docstrings, pylint

* still trying to fix docs

* still trying to fix the docs

* still trying to fix the docs

* still trying to fix the docs

* change qprogram to_readable_dict to __str__ method

* fix tests, fix docs

* add suggested changes

* add unittest for qprogram str method

* fix tests

* remove unused import

* fix coverage

* update changelog

* implementation proposal

* code quality

* testing annealing

* weights are optional

* raising error

* raises error test

* QHC 621 Modify set_intermediate_frequency for the jobAPI (#764)

* update to support opx1000

* fix import error

* fix typo

* add default type

* fix typo

* change octave connectivity

* Implemented necessary changes for QUA/job API (#756)

* Implemented necessary changes for QUA/job API

* Update tests/instruments/quantum_machines/test_quantum_machines_cluster.py

* FIXED AUTOMATIC FORMAT

* add if_outputs

* add if_outputs

* opx1000

* corrected deprecated functions

* fix typos and tests

* fix code quality

* Fixing Documentation warnings

* allowing more than a QM existing at same time

* Changes on the IF set

* updated job type

* ignored changes

* pylint disabled

* fix

* formatted tests

* Fixed isort issues

* fix pylint

* fix test

* fix

* added missing coverage

* fixed change

* Added octave calibrate_element after if / lo set

* fixed tests

* removed incorrect assertions

* fixed black formatter

* code quality

* code quality

* ignoring types

* black

* Ignoring job type

* Make ignore typing only for `[union-attr]`

* Rewritten job order to execute set_frequency

* changed from job to self.job for tests and the future

* Forgot a space :)

* ADDED PYLINT EXCEPTION

* Fixed Mypy

* a¡changed type requirements

* Added compatibility with opx+ and opx1000

* black

* reset self._intermediate_frequency

* fix test

* fixed moretests

* tests

* created function to get the controller

* black

* added changelog

* coverage

* fixed tests

* fix

* test reaise error

* pylint

* Update src/qililab/instruments/quantum_machines/quantum_machines_cluster.py

* Apply suggestions from code review

Co-authored-by: Vyron Vasileiadis <hi@fedonman.com>

* implemented suggested changes

* black

* fix tests

* removed unexistent test

---------

Co-authored-by: Vyron Vasileiads <hi@fedonman.com>
Co-authored-by: Guillermo Abad López <109400222+GuillermoAbadLopez@users.noreply.github.com>
Co-authored-by: GitHub Actions bot <actions@github.com>
Co-authored-by: jjmartinezQT <133863373+jjmartinezQT@users.noreply.github.com>

* tests

* isort

* measure in test

* code quality

* code quality

* removing duplicate fixture

* removing duplicate

* disabling too many lines in platform

* removing unused arguments

* calibration fixture

* code quality

* fixing unittest

* removing brackets typo

* passing by calibration to execute_qprogram

* refactoring

* fixing unittest

* removing comment

* [QHC-636] Create AnnealingSchedule class in Qililab. (#767)

* add parameter class and one qubit analog transpiler

* add unit tests

* add unit tests

* move fluqe parameter inside analog, fix pylint complains

* draft of anneal program with transpiler for 1q

* add analog program execution workflow

* add changelog

* fix mypy

* fix docstring

* add unit test for anneal program

* wip platform unit tests

* add to_dict method to qprogram in order to tests that a qprogram is equivalent to another

* fix tests

* fix pylint

* fix docstrings, pylint

* still trying to fix docs

* still trying to fix the docs

* still trying to fix the docs

* still trying to fix the docs

* change qprogram to_readable_dict to __str__ method

* fix tests, fix docs

* add suggested changes

* add unittest for qprogram str method

* fix tests

* remove unused import

* fix coverage

* update changelog

* tests

* returning changes lost

* isort

* measure in test

* Revert "measure in test"

This reverts commit 6dc9b07.

* Revert "isort"

This reverts commit a958ea8.

* Revert "returning changes lost"

This reverts commit 86a2bbd.

* Revert "tests"

This reverts commit d7d0669.

---------

Co-authored-by: GitHub Actions bot <actions@github.com>

* changelog

* code quality

* returning results of annealing

* returning type

* code quality

* making calibration file non optional

* introducing sync

* code quality

* code quality

* outside loop

* checking type

* changing test

* adding calibration

* fixing unittest

* checking return type

* removing src

* code quality

---------

Co-authored-by: victor <visangim@gmail.com>
Co-authored-by: GitHub Actions bot <actions@github.com>
Co-authored-by: jordivallsq <151619025+jordivallsq@users.noreply.github.com>
Co-authored-by: Vyron Vasileiads <hi@fedonman.com>
Co-authored-by: Guillermo Abad López <109400222+GuillermoAbadLopez@users.noreply.github.com>
@jjmartinezQT jjmartinezQT merged commit 1044924 into main Aug 21, 2024
6 checks passed
@jjmartinezQT jjmartinezQT deleted the qhc-622-adapt-the-qm-drivers-so-more-than-one-user-can-measure-at branch August 21, 2024 14:59
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.

6 participants