Allow to normalize inputs on StatePreparation and Initialize

[daniel-fry]

Summary

Details and comments

[daniel-fry]

The idea is that the user doesn't have to normalize their own input array and that by using normalize=True the user will understand that their input array needs to be normalized and that the Initializer will do it for them.

[Cryoris]

To fix the last errors you should run black, e.g. by calling make black in the terminal in the folder qiskit-terra. 🙂

[Cryoris]

@daniel-fry are you still pursuing this? 🙂

[daniel-fry]

@daniel-fry are you still pursuing this? 🙂

Hi @Cryoris, yes I'm still pursuing this and I hope to finish it soon (as my first contribution to Qiskit). I just saw your last message about make black. Will work on this today. Might have a couple of questions. Thanks again.

[daniel-fry]

@daniel-fry are you still pursuing this? 🙂

@Cryoris I can't find much for the "make black" command on Google. I assume it is coming from the black library for code style and I can follow this https://pypi.org/project/black/ ?

[mtreinish]

@daniel-fry are you still pursuing this? slightly_smiling_face

@Cryoris I can't find much for the "make black" command on Google. I assume it is coming from the black library for code style and I can follow this https://pypi.org/project/black/ ?

I would suggest just using tox to do this as is suggested in the contributing guide: https://github.com/Qiskit/qiskit-terra/blob/main/CONTRIBUTING.md#pull-request-checklist. Assuming you've installed tox into your python environment (if you haven't pip install tox will handle that) you just run tox -eblack to run black which will automatically fix the code formatting. You can also use tox -elint-incr to check the other style/lint enforcement that CI runs.

[daniel-fry]

@daniel-fry are you still pursuing this? slightly_smiling_face

@Cryoris I can't find much for the "make black" command on Google. I assume it is coming from the black library for code style and I can follow this https://pypi.org/project/black/ ?

I would suggest just using tox to do this as is suggested in the contributing guide: https://github.com/Qiskit/qiskit-terra/blob/main/CONTRIBUTING.md#pull-request-checklist. Assuming you've installed tox into your python environment (if you haven't pip install tox will handle that) you just run tox -eblack to run black which will automatically fix the code formatting. You can also use tox -elint-incr to check the other style/lint enforcement that CI runs.

OK thanks @mtreinish I have used tox and it reformatted the initiailizer.py file and gave me a "congratulations :)" but the check error is still there so I must have uploaded it wrong. Working on it...

[jakelishman]

I think your git client has its line-endings setting configured incorrectly for collaboration (don't worry, it's not a big deal). I'm guessing you're developing on Windows - if so, you can run

git config core.autocrlf true

in the repository and that will change your setting for this repository only to the right thing. Unless you have a good reason not to, you probably want to make this setting apply to all your repositories, which you do with

git config --global core.autocrlf true

The effect of this command is that git will make sure that the files have "Windows-style" line-endings when you're actually editing them, but they get converted to Linux/Mac style when you commit. That's the normal setup in projects with lots of people collaborating.

After you've changed your settings (either way), you can tell git to fix the file. You do that with

git add --renormalize qiskit/extensions/quantum_initializer/initializer.py
git commit -m "Fix line endings"

and then push that commit to GitHub.

So you don't have to just trust me to run random commands on your computer, here's the documentation from GitHub and from git itself about those settings.

[daniel-fry]

@jakelishman Thanks for the tip!

[daniel-fry]

Hi @Cryoris @jakelishman , I've done the things you suggested. My PR is failing only 1 check now I think. How do I better understand why it's failing? I can see something about azure pipeline...

[jakelishman]

At the bottom of this page, you should see a list of tests across Azure (the blue rocket logo) and GitHub Actions. You can click on "details" on any one of them, which takes you to a GitHub summary page, and the most useful information is found by clicking the "View more details on Azure Pipelines" link that's on this new page. That gets you to Azure, where if you scroll down you should be able to see the failing job(s). If you keep clicking through the red crosses, you'll get to the logs - in this case it's a failure to build the documentation, with the error

/home/vsts/work/1/s/qiskit/extensions/quantum_initializer/initializer.py:
docstring of qiskit.extensions.quantum_initializer.initializer.initialize:25:
Definition list ends without a blank line; unexpected unindent.

Sphinx (the docs tool) errors can be pretty unintelligible at times. In this case it's because of the slightly odd indentation in the docstring of Initializer.__init__. You can run the documentation build yourself locally with tox -e docs, to try things out - it can be a bit frustrating at first to learn reStructuredText (RST) because it's a surprisingly finnicky language, but it's powerful in the end.

(Getting to the logs is a bit fiddly for some reason - both GitHub and Azure insert a couple of unnecessary landing pages, so you have to click about four times when you'd expect it to only be one.)

---

Please could you also add some tests of the new behaviour, including any failure paths (e.g. ensuring that we test that if you pass an unnormalized vector without normalize=True, it errors). The failure-path tests might already be in place, but we've not historically been perfect about them, so they might not be.

[daniel-fry]

At the bottom of this page, you should see a list of tests across Azure (the blue rocket logo) and GitHub Actions. You can click on "details" on any one of them, which takes you to a GitHub summary page, and the most useful information is found by clicking the "View more details on Azure Pipelines" link that's on this new page. That gets you to Azure, where if you scroll down you should be able to see the failing job(s). If you keep clicking through the red crosses, you'll get to the logs - in this case it's a failure to build the documentation, with the error

/home/vsts/work/1/s/qiskit/extensions/quantum_initializer/initializer.py:
docstring of qiskit.extensions.quantum_initializer.initializer.initialize:25:
Definition list ends without a blank line; unexpected unindent.

Sphinx (the docs tool) errors can be pretty unintelligible at times. In this case it's because of the slightly odd indentation in the docstring of Initializer.__init__. You can run the documentation build yourself locally with tox -e docs, to try things out - it can be a bit frustrating at first to learn reStructuredText (RST) because it's a surprisingly finnicky language, but it's powerful in the end.

(Getting to the logs is a bit fiddly for some reason - both GitHub and Azure insert a couple of unnecessary landing pages, so you have to click about four times when you'd expect it to only be one.)

Please could you also add some tests of the new behaviour, including any failure paths (e.g. ensuring that we test that if you pass an unnormalized vector without normalize=True, it errors). The failure-path tests might already be in place, but we've not historically been perfect about them, so they might not be.

Thanks for the great advice and tips. I appreciate it a lot.
Could you send me a link for how to add a failure path test please? You mean something besides adding code to make sure it errors?

[jakelishman]

Yeah, exactly. In this case, you expect that if you try to pass an unnormalised array (like np.array([1, 2, 3])) with normalize=False, then it'll raise QiskitError. The test is written something like

def test_initialize_errors_on_unnormalized_input(self):
    qc = QuantumCircuit(3)
    with self.assertRaisesRegex(QiskitError, "<part of the expected message>"):
        qc.initialize(np.array([1, 2, 3]), normalize=False)

The test will report a failure if that type of exception isn't raised, or if the message doesn't match. The documentation for unittest.TestCase.assertRaisesRegex is here.

[daniel-fry]

@Cryoris @jakelishman Unfortunately I don't have the time to make progress on this and understand how to push it through. Could you take over this please?
If you could write a few sentences after to explain how you resolved the issues I would greatly greatly appreciate it! and may be able to finish next time. Thank you for your help.

[coveralls]

Pull Request Test Coverage Report for Build 5323977910

• 13 of 13 (100.0%) changed or added relevant lines in 2 files are covered.
• 7 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-0.006%) to 85.97%

---

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	1	91.14%
crates/qasm2/src/parse.rs	6	97.58%

Totals
Change from base Build 5320987396:	-0.006%
Covered Lines:	71480
Relevant Lines:	83145

---

💛 - Coveralls

[Cryoris]

It would probably be good to hold this off until #7666 is merged, since this here is the smaller change and will likely be easier to integrate 🙂

[Cryoris]

This is not blocked anymore, so we can proceed if you're still interested @daniel-fry! Since Initialize was split partially into StatePreparation, the functionality would probably have to be moved there 🙂

[daniel-fry]

This is not blocked anymore, so we can proceed if you're still interested @daniel-fry! Since Initialize was split partially into StatePreparation, the functionality would probably have to be moved there 🙂

Hi @Cryoris ,
I think the code itself is complete. Thanks for all your help.

Unfortunately I don't have time to push it through the quality checks. Will you be able to do this? If there are any blocks I'd be grateful if you could tell me what they are and how to overcome them for next time.