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

ExperimentData tests assume that .data() is ordered #1044

Closed
wshanks opened this issue Feb 13, 2023 · 0 comments · Fixed by #1267
Closed

ExperimentData tests assume that .data() is ordered #1044

wshanks opened this issue Feb 13, 2023 · 0 comments · Fixed by #1267
Labels
bug Something isn't working
Milestone

Comments

@wshanks
Copy link
Collaborator

wshanks commented Feb 13, 2023

Informations

  • Qiskit Experiments version: c0cc88c
  • Python version: All
  • Operating system: All

What is the current behavior?

Several of the ExperimentData tests assume that ExperimentData.data() returns data in a reliable order. This is not the case.

Steps to reproduce the problem

One example is this test:

https://github.com/Qiskit/qiskit-experiments/blob/c0cc88c244bc4e41fd3355d76ff2438a10dbac43/test/database_service/test_db_experiment_data.py#L119-L140

It can fail as was the case here.

The problem is that ExperimentData puts jobs that are added to it into a thread pool that waits for their results to be available and then adds them to an internal list that holds all the results. The test adds jobs very quickly and apparently sometimes the second job can come out of the thread pool before the first.

What is the expected behavior?

The tests should not rely on the order of data returned by ExperimentData.data().

Suggested solutions

In practice, the analysis code usually attaches labels to the metadata of each circuit which is then attached to each result. The data is then analyzed using these labels, so the order of the data returned by data() does not matter. One solution would be to add labels to the fake results used in the tests and then sort by those labels before comparing to the expected results.

@wshanks wshanks added the bug Something isn't working label Feb 13, 2023
@coruscating coruscating added this to the Release 0.6 milestone Mar 29, 2023
wshanks added a commit to wshanks/qiskit-experiments that referenced this issue Sep 7, 2023
Previously, the test `test_add_data_job` called `add_jobs` with two jobs
and then tested that the results from `ExperimentData.data()` matched
the results of the first job concatenated with the second.
`ExperimentData` uses threads to handle tracking jobs. Even though these
jobs are dummy jobs that take no time, `ExperimentData`  would
occasionally add the results from the second job before the first and
then fail the results comparison. With this change, a label is attached
to each result and then the results are sorted based on the label
before testing against the expected results.

Closes qiskit-community#1044
github-merge-queue bot pushed a commit that referenced this issue Sep 7, 2023
Previously, the test `test_add_data_job` called `add_jobs` with two jobs
and then tested that the results from `ExperimentData.data()` matched
the results of the first job concatenated with the second.
`ExperimentData` uses threads to handle tracking jobs. Even though these
jobs are dummy jobs that take no time, `ExperimentData` would
occasionally add the results from the second job before the first and
then fail the results comparison. With this change, a label is attached
to each result and then the results are sorted based on the label before
testing against the expected results.

Closes #1044
nkanazawa1989 pushed a commit to nkanazawa1989/qiskit-experiments that referenced this issue Jan 10, 2024
…munity#1267)

Previously, the test `test_add_data_job` called `add_jobs` with two jobs
and then tested that the results from `ExperimentData.data()` matched
the results of the first job concatenated with the second.
`ExperimentData` uses threads to handle tracking jobs. Even though these
jobs are dummy jobs that take no time, `ExperimentData` would
occasionally add the results from the second job before the first and
then fail the results comparison. With this change, a label is attached
to each result and then the results are sorted based on the label before
testing against the expected results.

Closes qiskit-community#1044
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants