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

Create visualization tests for pulse #6502

Open
enavarro51 opened this issue Jun 3, 2021 · 15 comments
Open

Create visualization tests for pulse #6502

enavarro51 opened this issue Jun 3, 2021 · 15 comments
Assignees
Labels
good first issue Good for newcomers type: enhancement It's working, but needs polishing

Comments

@enavarro51
Copy link
Contributor

What is the expected enhancement?

As part of #6467, some old pulse image comparison tests that were being skipped were removed. These were in the file test_pulse_visualization_output.txt. The tests in this file may be useful when a #4544 binder style set of image comparison tests is created for pulse.

@enavarro51 enavarro51 added the type: enhancement It's working, but needs polishing label Jun 3, 2021
@taalexander taalexander added the good first issue Good for newcomers label Jun 9, 2021
@MartinBeseda
Copy link

Dear @enavarro51 ,
Dear @taalexander ,

I'd have a look at this, if possible.

@javabster
Copy link
Contributor

hey @MartinBeseda sorry for the slow response, are you still interested in working on this? If yes lmk and I'll assign to you 😄

@ozamanan
Copy link

Hey, @javabster can I work on this?

@javabster
Copy link
Contributor

Hi @ozamanan as @MartinBeseda is unresponsive I will assign to you, let us know if you have any questions 😄

@anonymousr007
Copy link

Hi @javabster, Add text_pulse_visualization_output.txt in this folder /test/python/pulse. Right ?

@javabster
Copy link
Contributor

Hi @javabster, Add text_pulse_visualization_output.txt in this folder /test/python/pulse. Right ?

Hi @anonymousr007 actually I think the point of this issue is to create some binder style snapshot tests for pulse, using the tests found in text_pulse_visualization_output.txt. The other binder tests can be found in test/ipynb, I would suggest creating a subfolder test/ipynb/pulse and following the same format that is used for the other circuit and graph tests.

@1ucian0
Copy link
Member

1ucian0 commented Jan 31, 2022

Any news @ozamanan ?

@1ucian0
Copy link
Member

1ucian0 commented May 25, 2022

Hi @anonymousr007
The assigned contributor is unresponsive are you interested in taking over?

@AbdullahKazi500
Copy link

The expected enhancement is to create a set of image comparison tests for pulse, similar to the existing #4544 binder style tests. These tests will help ensure that the pulse images generated by different pulse programs are consistent and correct. The tests can be created based on the old pulse image comparison tests that were previously skipped, which were removed as part of #6467. By adding these tests back in a new form, the pulse functionality can be more thoroughly tested and validated @1ucian0 Can I Get assigned to this issue

@1ucian0
Copy link
Member

1ucian0 commented Apr 16, 2023

Thanks @AbdullahKazi500 ! assigning.

Small headsup. The binder mechanism is growing intro proper CI testing in #9961 . There are two options at this point:

  • You can write pulse tests top of AngeloDanducci:ad9892. This creates a dependency and a potential blocker, but it does not need follow up.
  • You can do it in the notebook way and then we moved them to the CI way. This allows you to work more independently, but it might requiere an update if 9961 lands before yours.

Up to you. Whatever you feel more confortable with.

@AbdullahKazi500
Copy link

Thank you for the heads up! It's good to know that the binder mechanism is moving towards proper CI testing. I'll keep these options in mind when writing pulse tests. It seems like using the notebook way first and then moving to CI testing would be the safer option in case there are any changes in #9961.

@Raghav-Bell
Copy link
Contributor

Hi @1ucian0! It seems #9961 is merged, binder tests are integrated in the workflow. Is this issue still open ?

@AbdullahKazi500
Copy link

Hi @Raghav-Bell Yeah I am working on this for now

@1ucian0 1ucian0 changed the title Create binder styles tests for pulse Create visualization tests for pulse Sep 16, 2023
@1ucian0
Copy link
Member

1ucian0 commented Sep 16, 2023

Hi @AbdullahKazi500 How is that going?

@AbdullahKazi500
Copy link

@1ucian0 Hi Luciano I have created a Jupyter Notebook-based tests for pulse that run as part of the Continuous Integration (CI) process. These notebooks should include the necessary test cases and comparisons.
Once I have implemented the initial CI tests, I will try to create a new branch in your fork of the Qiskit repository,
I am trying to Locate the Tests and Review the contents of the file test_pulse_visualization_output.txt in the Qiskit repository to understand the older image comparison tests that were removed.
whether these tests are still relevant and whether they can be incorporated into the new CI testing framework for pulse.
Depending on their relevance and the current state of the code, I Can reimplement these tests or adapt them to the new testing framework.
If necessary, I can create new tests based on the older tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers type: enhancement It's working, but needs polishing
Projects
Status: Assigned
Development

No branches or pull requests

9 participants