Use allow_unknown_parameters in SparseLabelOp.assign_parameters

[kevinsung]

Summary

Fixes #1010

Details and comments

[coveralls]

Pull Request Test Coverage Report for Build 3967567123

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.01%) to 85.901%

---

Totals
Change from base Build 3960912582:	0.01%
Covered Lines:	17602
Relevant Lines:	20491

---

💛 - Coveralls

[mrossinek]

In general I think this is good to go. I am just not sure about some "meta-details".
What I mean by that is that this refactors the change done by the bugfix PR #1008. That PR was backported to stable (but not released yet).

• Does that mean we can simply merge this as is without a release note?
• Should we backport this, too?

Pinging @woodsp-ibm and @manoelmarques for input on this

[mrossinek]

Actually, this now uses an unreleased feature of Qiskit Terra so we canNOT backport it as such. I am a bit surprised that the CI did not catch this... @manoelmarques are we still testing against Qiskit Terra installed from source even though our requirements file currently only uses 0.22.*?

See comment above

[woodsp-ibm]

If the change uses/depends on new features from an as yet unreleased version then the requirements.txt needs to get updated too by this PR, if it does not already reflect the version dependence it requires.

[kevinsung]

If the change uses/depends on new features from an as yet unreleased version then the requirements.txt needs to get updated too by this PR, if it does not already reflect the version dependence it requires.

Yes, I can do that. But I think Max is asking why this PR passed the CI even though I did not update requirements.txt, considering that it does use an unreleased feature in Terra.

[mrossinek]

Yes, I can do that. But I think Max is asking why this PR passed the CI even though I did not update requirements.txt, considering that it does use an unreleased feature in Terra.

I am indeed wondering that. Can you, thus, please ensure the following:

• update requirements.txt
• ensure there is a test which actually triggers this case

[kevinsung]

The test for this change was added in the previous PR, #1008.

Do you want me to update requirements.txt now, or do you want to wait to fix the CI issue so this PR can be used to test that the CI is working properly?

[mrossinek]

The test for this change was added in the previous PR, #1008.

The test there does not seem to be actually doing this though. It constructs an operator which will have two parameters, a and b, and then assigns a value to a. If I understand this change correctly, the method is supposed to work correctly even if one were to provide a parameter called for example c, which is unknown to the operator.

Do you want me to update requirements.txt now, or do you want to wait to fix the CI issue so this PR can be used to test that the CI is working properly?

If my reasoning above is correct, the CI is working fine and this case is not covered correctly by the unittests.

[kevinsung]

The test there does not seem to be actually doing this though. It constructs an operator which will have two parameters, a and b, and then assigns a value to a.

The test there was actually testing this behavior because it causes the ParameterExpression containing only b to be passed an assignment dictionary only containing a. Nevertheless, I've added another test on your suggestion that is perhaps more explicit.

[mrossinek]

Alright, then all that is missing is the update of the requirements.txt file. Please do that here, too

[kevinsung]

Do you want me to update requirements.txt now, or do you want to wait to fix the CI issue so this PR can be used to test that the CI is working properly?

[mrossinek]

As explicitly said: please update it now.

We are continuing to test Qiskit Nature against Qiskit Terra main to catch potential breakage early. This is an intentional choice of the application developers team to do this for the packages within the Qiskit organization. Thus, CI is indeed working as intended.

The reason I originally expected this to fail would have been the neko tests, but this simply does not catch this particular change which is not an issue we can fix here (nor wait to be fixed).

[kevinsung]

What's the correct way to update requirements.txt? I tried the following, neither of which worked:

• qiskit-terra>=0.23.*
• git+https://github.com/Qiskit/qiskit-terra@main#egg=qiskit-terra

[manoelmarques]

Currently it is qiskit-terra>=0.22.* so it should just bump to qiskit-terra>=0.23.*
I saw your CI and the failure was on Neko. This is very unfortunate since the idea behind this Neko thing is to test against Terra from Pypi.
I have no control over Neko since it was created by Terra. For Neko to work, since Pypi Terra 0.23 is not yet released, you cannot bump it until there is a 0.23 (0.23rc1) version in Pypi which should be this week

ERROR: Could not find a version that satisfies the requirement qiskit-terra>=0.23.* (from qiskit-nature) (from versions: 0.7.0, 0.7.1, 0.7.2, 0.8.0, 0.8.1, 0.8.2, 0.9.0, 0.9.1, 0.10.0, 0.11.0, 0.11.1, 0.12.0, 0.13.0, 0.14.0, 0.14.1, 0.14.2, 0.15.0, 0.15.1, 0.15.2, 0.16.0, 0.16.1, 0.16.2, 0.16.3, 0.16.4, 0.17.0, 0.17.1, 0.17.2, 0.17.3, 0.17.4, 0.18.0, 0.18.1, 0.18.2, 0.18.3, 0.19.0, 0.19.1, 0.19.2, 0.20.0, 0.20.1, 0.20.2, 0.21.0rc1, 0.21.0, 0.21.1, 0.21.2, 0.22.0rc1, 0.22.0, 0.22.1, 0.22.2, 0.22.3, 0.22.4)
ERROR: No matching distribution found for qiskit-terra>=0.23.*