qiskit-community / qiskit-aqua

Quantum Algorithms & Applications (**DEPRECATED** since April 2021 - see readme for more info)
https://qiskit.org/aqua
Apache License 2.0
574 stars 376 forks source link

Update qiskit versions of requirements #1571

Closed t-imamichi closed 3 years ago

t-imamichi commented 3 years ago

Summary

Fix the CI error of terra.

The following causes an error:

$ pip install "git+https://github.com/Qiskit/qiskit-ignis" "git+https://github.com/Qiskit/qiskit-aqua"
Collecting git+https://github.com/Qiskit/qiskit-ignis
  Cloning https://github.com/Qiskit/qiskit-ignis to /private/var/folders/vt/58nt4rrd4mz552nj2l96bbnm0000gn/T/pip-req-build-e8ryl8mw
  Running command git clone -q https://github.com/Qiskit/qiskit-ignis /private/var/folders/vt/58nt4rrd4mz552nj2l96bbnm0000gn/T/pip-req-build-e8ryl8mw
Collecting git+https://github.com/Qiskit/qiskit-aqua
  Cloning https://github.com/Qiskit/qiskit-aqua to /private/var/folders/vt/58nt4rrd4mz552nj2l96bbnm0000gn/T/pip-req-build-vtftzx3q
  Running command git clone -q https://github.com/Qiskit/qiskit-aqua /private/var/folders/vt/58nt4rrd4mz552nj2l96bbnm0000gn/T/pip-req-build-vtftzx3q
Requirement already satisfied: qiskit-terra<0.18.0,>=0.17.0 in /Users/ima/envs/dev38/lib/python3.8/site-packages (from qiskit-aqua==0.10.0) (0.17.0)
ERROR: Cannot install qiskit-aqua==0.10.0 and qiskit-ignis 0.7.0 (from git+https://github.com/Qiskit/qiskit-ignis) because these package versions have conflicting dependencies.

The conflict is caused by:
    The user requested qiskit-ignis 0.7.0 (from git+https://github.com/Qiskit/qiskit-ignis)
    qiskit-aqua 0.10.0 depends on qiskit-ignis<0.7.0 and >=0.6.0

To fix this you could try to:
1. loosen the range of package versions you've specified
2. remove package versions to allow pip attempt to solve the dependency conflict

ERROR: ResolutionImpossible: for help visit https://pip.pypa.io/en/latest/user_guide/#fixing-conflicting-dependencies

See https://github.com/Qiskit/qiskit-terra/pull/6160 CI log: https://dev.azure.com/qiskit-ci/qiskit-terra/_build/results?buildId=26430&view=logs&jobId=47acf835-66bb-5b0f-07e4-6f53ffe80e20&j=47acf835-66bb-5b0f-07e4-6f53ffe80e20&t=d6cc4d9a-83df-59b0-cfe4-173c81c4d987

Details and comments

manoelmarques commented 3 years ago

I thought qiskit-aqua was supposed to stay frozen to the current qiiskit-terra, qiskit-ignis in pypi forever. If we extend the requirements to the latest version in master, then aqua will break eventually and we will have to fix frozen code. I will soon create a PR for qiskit-tutorials eliminating all aqua usage there. I would be ok with this change here until qiskit-tutorials doesn’t use aqua anymore but once it changes, I believe terra should remove aqua from its CI and we should go back to the previous requirements.

t-imamichi commented 3 years ago

I'm OK to fix terra. Anyway we need to fix these lines, otherwise terra CI does not pass. https://github.com/Qiskit/qiskit-terra/blob/e1d49102dd18bf6ffdf28e5657e1cde4091dc609/azure-pipelines.yml#L709-L710

manoelmarques commented 3 years ago

Yes, that is why I am ok with this PR being merged but then reverted once aqua is not a dependency in qiskit-tutorials and then they should remove aqua from their CI and we should revert. Currently, I agree I don't see another way because even if they try to use aqua from pypi they will get the same error. The only way I see to fix terra CI while aqua is there would be to not use anything from master in their CI for qiskit-tutorials. I am not sure they would agree to that change.

This seems to be an emergency in Terra as all their PRs are failing because of this right? I will approve this fix for now.

t-imamichi commented 3 years ago

Yes, https://github.com/Qiskit/qiskit-terra/pull/6160 fails even if it fixes only a typo. This is the CI log https://dev.azure.com/qiskit-ci/qiskit-terra/_build/results?buildId=26430&view=logs&jobId=47acf835-66bb-5b0f-07e4-6f53ffe80e20&j=47acf835-66bb-5b0f-07e4-6f53ffe80e20&t=d6cc4d9a-83df-59b0-cfe4-173c81c4d987

t-imamichi commented 3 years ago

Could you double check whether this PR is enough to fix the terra CI issue?

manoelmarques commented 3 years ago

It should fix the current version conflict with ignis and terra there. The aqua code passes their tutorias because the aqua CI runs their tutorials too as double check. The difference is that our CI reverts to terra, ignis pypi if master cannot satisfy our requirements.. That is why our CI still works.

As far as other problems down the line is harder to say if there are other sub-dependencies that may cause conflicts without running their CI.

t-imamichi commented 3 years ago

Thank you for your information. I finally understand why aqua CI works fine even if terra CI fails.

manoelmarques commented 3 years ago

Yes, in this PR I might have just extended the upper bounds and leave the lower bounds but if it will be reverted later, it is not so important.

t-imamichi commented 3 years ago

I see. I adjust this PR as you suggest. What do you think?

manoelmarques commented 3 years ago

Yes, just change the upper bounds as to leave it for now installable with pypi terra/ignis as well as master. The current change doesn't let you use terra/ignis from pypi even though aqua works fine with them.

t-imamichi commented 3 years ago

Thanks. I reverted the lower bounds.