allows string values in assign_parameters with strict=False #14365

kaldag · 2025-05-15T05:26:06Z

Summary

Allows for string values as arguments in assign_parameters with strict=False.
Fixes #13933

Details and comments

qiskit-bot · 2025-05-15T05:26:13Z

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the following people are relevant to this code:

@Qiskit/terra-core

mtreinish

Thanks for tackling this. I left a comment inline, can you also add a release note and some unit tests to confirm the fix and prevent it from regressing in the future?

mtreinish · 2025-05-21T17:38:53Z

qiskit/circuit/quantumcircuit.py

+            elif isinstance(parameter, str) and not self.has_parameter(parameter):
+                out[Parameter(parameter)] = value


I assume you did this to keep the check in place for the error message when strict=True in the calling function. It's just a bit odd looking here because I'd expect we'd just skip above. I'd be tempted to maybe move the extras logic into this function and that would simplify the logic a bit. So add strict as an argument to this function and collect extras for an error message if it's set here.

jakelishman

Thanks for looking at this Kalyan, sorry it took me awfully long to respond first time round. I left a comment on the implementation, and additionally:

please can you add a test case to the suite as a regression test? I think the relevant file is test/python/circuit/test_parameters.py, but it's worth checking.
please can you add a release note about the bug fix? There's instructions on release notes here

jakelishman · 2025-05-21T17:40:13Z

qiskit/circuit/quantumcircuit.py

-            elif isinstance(parameter, str):
+            elif isinstance(parameter, str) and self.has_parameter(parameter):
                out[self.get_parameter(parameter)] = value
+            elif isinstance(parameter, str) and not self.has_parameter(parameter):
+                out[Parameter(parameter)] = value


If the QuantumCircuit doesn't have the parameter, I don't think there's any benefit making a dummy Parameter instance - it'll never bind to anything anyway. I think we can just not add it to the dictionary if it's not present and we're in strict=False mode, but we still need to throw an error if we're in strict=True. We might need to thread a strict keyword argument down to this inner method.

@jakelishman @mtreinish Thanks for the suggestions. However, even if we pass strict to _unroll_param_dict, the fact that a dictionary with string keys gets passed to target._data.assign_parameters_mapping(raw_mapping) will remain a problem, unless we convert the keys to Parameter type. If the parameters are already present in the circuit, it is not a problem because the method get_parameter converts the keys from string type to Parameter type. However, if the parameters are not present in the circuit, we will have to create a Parameter instance out of the strings.

Once it is passed as a dictionary with Parameter keys to the assign_parameters method, the rest of the logic falls in place. We don't really need to pass strict to the _unroll_param_dict method.

Changed as suggested by @jakelishman. strict argument passed to the inner method.

coveralls · 2025-05-23T11:44:21Z

Pull Request Test Coverage Report for Build 15473811754

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

4 of 5 (80.0%) changed or added relevant lines in 1 file are covered.
20 unchanged lines in 3 files lost coverage.
Overall coverage decreased (-0.004%) to 87.976%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/circuit/quantumcircuit.py	4	5	80.0%

Files with Coverage Reduction	New Missed Lines	%
crates/circuit/src/symbol_expr.rs	4	73.77%
crates/qasm2/src/lex.rs	4	92.23%
crates/qasm2/src/parse.rs	12	97.15%

Totals
Change from base Build 15473244251:	-0.004%
Covered Lines:	83021
Relevant Lines:	94368

💛 - Coveralls

jakelishman

Thanks Kalyan - I pushed up a small commit to make the test case more rigorous (testing that the result was correct by comparison to a known-good call), but otherwise the changeset is good.

I'll backport this into all supported branches - it'll go out in 1.4.4 and (at least) 2.1.0 final.

jakelishman · 2025-06-06T14:11:52Z

@Mergifyio backport stable/1.4 stable/2.1

mergify · 2025-06-06T14:12:26Z

backport stable/1.4 stable/2.1

✅ Backports have been created

#14553 allows string values in assign_parameters with strict=False (backport #14365) has been created for branch stable/1.4
#14554 allows string values in assign_parameters with strict=False (backport #14365) has been created for branch stable/2.1

* allows string values in assign_parameters with strict=False * added release notes and a test case * formatting changes in unittest file * Changes with `strict` passed to an inner method. * formatting changes in release notes * corrections in inner method * Make test more rigorous --------- Co-authored-by: Jake Lishman <jake.lishman@ibm.com> (cherry picked from commit 25dec6a)

…14554) * allows string values in assign_parameters with strict=False * added release notes and a test case * formatting changes in unittest file * Changes with `strict` passed to an inner method. * formatting changes in release notes * corrections in inner method * Make test more rigorous --------- (cherry picked from commit 25dec6a) Co-authored-by: kaldag <kalyan.dasgupta@gmail.com> Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

…14553) * allows string values in assign_parameters with strict=False * added release notes and a test case * formatting changes in unittest file * Changes with `strict` passed to an inner method. * formatting changes in release notes * corrections in inner method * Make test more rigorous --------- (cherry picked from commit 25dec6a) Co-authored-by: kaldag <kalyan.dasgupta@gmail.com> Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

…4365) * allows string values in assign_parameters with strict=False * added release notes and a test case * formatting changes in unittest file * Changes with `strict` passed to an inner method. * formatting changes in release notes * corrections in inner method * Make test more rigorous --------- Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

kaldag requested a review from a team as a code owner May 15, 2025 05:26

qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label May 15, 2025

github-project-automation bot added this to Contributor Monitoring May 15, 2025

mtreinish reviewed May 21, 2025

View reviewed changes

jakelishman reviewed May 21, 2025

View reviewed changes

eliarbel removed the Community PR PRs from contributors that are not 'members' of the Qiskit repo label May 29, 2025

kaldag added 5 commits June 3, 2025 13:54

allows string values in assign_parameters with strict=False

a83723b

added release notes and a test case

f01ce75

formatting changes in unittest file

0cfa370

Changes with strict passed to an inner method.

b2a2562

formatting changes in release notes

4e4d811

kaldag force-pushed the assign_params_strict branch from 7faa378 to 4e4d811 Compare June 3, 2025 08:24

kaldag and others added 6 commits June 3, 2025 13:54

corrections in inner method

6fa0ca4

Merge branch 'main' into assign_params_strict

26a0dc5

Merge branch 'main' into assign_params_strict

ae68757

Merge branch 'main' into assign_params_strict

913f4d6

Merge branch 'main' into assign_params_strict

cbaadba

Make test more rigorous

82e438f

jakelishman approved these changes Jun 6, 2025

View reviewed changes

jakelishman enabled auto-merge June 6, 2025 14:11

jakelishman added Changelog: Bugfix Include in the "Fixed" section of the changelog mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library labels Jun 6, 2025

jakelishman added this pull request to the merge queue Jun 6, 2025

Merged via the queue into Qiskit:main with commit 25dec6a Jun 6, 2025
27 checks passed

github-project-automation bot moved this to Done in Contributor Monitoring Jun 6, 2025

This was referenced Jun 6, 2025

allows string values in assign_parameters with strict=False (backport #14365) #14553

Merged

allows string values in assign_parameters with strict=False (backport #14365) #14554

Merged

kaldag deleted the assign_params_strict branch June 6, 2025 19:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

allows string values in assign_parameters with strict=False #14365

allows string values in assign_parameters with strict=False #14365

Uh oh!

kaldag commented May 15, 2025

Uh oh!

qiskit-bot commented May 15, 2025

Uh oh!

mtreinish left a comment

Uh oh!

mtreinish May 21, 2025

Uh oh!

jakelishman left a comment

Uh oh!

jakelishman May 21, 2025

Uh oh!

kaldag May 23, 2025 •

edited

Loading

Uh oh!

kaldag May 29, 2025

Uh oh!

coveralls commented May 23, 2025 •

edited

Loading

Uh oh!

jakelishman left a comment

Uh oh!

jakelishman commented Jun 6, 2025

Uh oh!

mergify bot commented Jun 6, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

		elif isinstance(parameter, str) and not self.has_parameter(parameter):
		out[Parameter(parameter)] = value

allows string values in assign_parameters with strict=False #14365

allows string values in assign_parameters with strict=False #14365

Uh oh!

Conversation

kaldag commented May 15, 2025

Summary

Details and comments

Uh oh!

qiskit-bot commented May 15, 2025

Uh oh!

mtreinish left a comment

Choose a reason for hiding this comment

Uh oh!

mtreinish May 21, 2025

Choose a reason for hiding this comment

Uh oh!

jakelishman left a comment

Choose a reason for hiding this comment

Uh oh!

jakelishman May 21, 2025

Choose a reason for hiding this comment

Uh oh!

kaldag May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaldag May 29, 2025

Choose a reason for hiding this comment

Uh oh!

coveralls commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 15473811754

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

jakelishman left a comment

Choose a reason for hiding this comment

Uh oh!

jakelishman commented Jun 6, 2025

Uh oh!

mergify bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Backports have been created

Uh oh!

Uh oh!

Uh oh!

kaldag May 23, 2025 •

edited

Loading

coveralls commented May 23, 2025 •

edited

Loading

mergify bot commented Jun 6, 2025 •

edited

Loading