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

Tutorial: Deuteron binding energy #580

Merged

Conversation

jvscursulim
Copy link
Contributor

Summary

A tutorial about how to use Qiskit Nature tools to tackle the specific problem of deuteron binding energy.

Details and comments

This PR is linked with the Issue #559. In this Issue, was proposed the idea of add a new tutorial in Qiskit Nature to explain how to use FermionicOp and others tools to determine the value of the binding energy between a proton and a neutron in the Deuteron nucleus, as was described in https://arxiv.org/pdf/1801.03897.pdf.

@CLAassistant
Copy link

CLAassistant commented Feb 25, 2022

CLA assistant check
All committers have signed the CLA.

@woodsp-ibm
Copy link
Member

In regards of the the present CI failures here.

  1. One is that all code here should be formatted with black and this includes notebooks. This can be done with 'make black' in you project root that covers the whole repo and will change what is needed.

  2. The other, I expected, this is because we run spell checks and often names and other new technical words need adding to the custom dictionary - its the .pylintdict file in the project root which as you will see is a list, in alphabetic order, of all the custom words we have so far. New ones would be added in the right place in lowercase as per the existing ones.

One final note is that all the tutorials should carry a version and copyright at the end produced from a cell like this (you can see this in the existing tutorials).

import qiskit.tools.jupyter

%qiskit_version_table
%qiskit_copyright

@jvscursulim
Copy link
Contributor Author

About the problems

  1. I reformatted the jupyter notebook with black.
  2. I included some new technical words in the .pydictlint file. (annihilation, deuteron, deuterium, eta and others)
  3. Added a cell in the notebook with the version table and copyright.

I'm uploading the updated files, hope this time everything works.

…d words pointed out by the spell check in the pydictlint file.
@woodsp-ibm
Copy link
Member

For the Dirac notation isn't it usually bra-ket with a hyphen in there? Which would pass the spell check without the custombraket addition I think since since bra would be a known word and ket is already there as a custom one.

@jvscursulim
Copy link
Contributor Author

For the Dirac notation isn't it usually bra-ket with a hyphen in there? Which would pass the spell check without the custombraket addition I think since since bra would be a known word and ket is already there as a custom one.

Yeah, that is true! There is a hyphen in there, I just added the word without thinking about the typo, sorry about that. I'll fix this in a new commit.

@coveralls
Copy link

coveralls commented Feb 26, 2022

Pull Request Test Coverage Report for Build 2845100980

  • 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.305%

Totals Coverage Status
Change from base Build 2842472941: 0.01%
Covered Lines: 17572
Relevant Lines: 20599

💛 - Coveralls

Copy link
Member

@mrossinek mrossinek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have time to go through the entire tutorial but at least I got started
with this. I left some more or less detailed comments below. Some of them can be
extended towards sections which I did not yet review in detail. Particular the
explanations should be proof-read.

I am wondering whether we can find a better way to actually compute the energy.
Here, you are constructing a function which creates and runs the VQE but there
you are not really making any use of Qiskit Nature.
The GroundStateEigensolver would come to mind here but I am aware that it has
the "limitation" of taking a Problem instance as its input to solve(). But
maybe constructing a DummyProblem would be a shorter and more Nature-oriented
way of doing things, than the current implementation of the
compute_binding_energy method.

Just wanted to put this out there for you to consider.

@jvscursulim
Copy link
Contributor Author

@mrossinek Thanks for your comments and suggestions, I'll work to make these changes as soon as possible.

@1ucian0 1ucian0 linked an issue Jun 15, 2022 that may be closed by this pull request
@mrossinek mrossinek removed the request for review from pbark June 29, 2022 15:26
@HuangJunye HuangJunye added the Community PR PRs from contributors that are not 'members' of the Qiskit organization label Jun 30, 2022
Copy link
Member

@mrossinek mrossinek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the long delay on reviewing your PRs!

We just merged #746 which majorly restructured the layout of our code base.
Effectively, all parts of the code which you are using in this tutorial are now residing under qiskit_nature/second_q. Please update your tutorial accordingly.

I will soon get around to doing a more in-depth review of this tutorial! 👍

@jvscursulim
Copy link
Contributor Author

Sorry for the long delay on reviewing your PRs!

We just merged #746 which majorly restructured the layout of our code base. Effectively, all parts of the code which you are using in this tutorial are now residing under qiskit_nature/second_q. Please update your tutorial accordingly.

I will soon get around to doing a more in-depth review of this tutorial! 👍

No problem and thanks for letting me know about the changes due #746, I will update the code according to these changes.

I'll wait for more comments and suggestions.

Copy link
Member

@mrossinek mrossinek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I finally managed to do a full review.
There are quite a lot of comments here, so please let me know if I need to clarify anything further.

Overall, I think this is a nice tutorial. There are some aspects which need to be changed but you can see the comments below for more details.

At first, I was wondering about the first person perspective of the writing style. Maybe an impersonal perspective would be better suited to guide a user through this, but you can also leave it as it is now.

If you disagree with my comments (especially the one about the Qiskit Terra aspects), maybe the Qiskit Community Tutorials (https://github.com/qiskit-community/qiskit-community-tutorials) are a better place for this. But let's see how this progresses 👍

@mrossinek
Copy link
Member

I think the new changes look very good 👍
I left some replies in the threads above and I cannot seem to find a comment which I thought I had left yesterday, so I'll repeat it here:
I think it would be nice, if instead of having one plot after the other (which makes the tutorial very long and makes it impossible to view plots all at once), they were done as a group of subplots. So e.g. the four SLSQP plots could be arranged in four quadrants. The same could be done for the COBYLA plots.

And one final comment w.r.t. writing: ansatz should basically always go with an article. You frequently omit that. I tried correcting this yesterday but I suggest you read through everything again and make sure those articles are in place 👍

@jvscursulim
Copy link
Contributor Author

I think the new changes look very good 👍 I left some replies in the threads above and I cannot seem to find a comment which I thought I had left yesterday, so I'll repeat it here: I think it would be nice, if instead of having one plot after the other (which makes the tutorial very long and makes it impossible to view plots all at once), they were done as a group of subplots. So e.g. the four SLSQP plots could be arranged in four quadrants. The same could be done for the COBYLA plots.

And one final comment w.r.t. writing: ansatz should basically always go with an article. You frequently omit that. I tried correcting this yesterday but I suggest you read through everything again and make sure those articles are in place 👍

I changed the plots, now I'm using subplots with nrows = 3 for COBYLA and nrows = 4 for SLSQP plots. I hope that now is better to observe the plots.

mrossinek
mrossinek previously approved these changes Aug 11, 2022
Copy link
Member

@mrossinek mrossinek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the great work on this! I think this tutorial is now in very good shape! Maybe some phrasing could be improved here and there, but I think this will always be the case to some degree.

So I will tentatively approve this and will give @woodsp-ibm a chance to review this before merging anything 👍

Again, thank you @jvscursulim, for the great and patient work here!

@jvscursulim
Copy link
Contributor Author

Thanks for the great work on this! I think this tutorial is now in very good shape! Maybe some phrasing could be improved here and there, but I think this will always be the case to some degree.

So I will tentatively approve this and will give @woodsp-ibm a chance to review this before merging anything 👍

Again, thank you @jvscursulim, for the great and patient work here!

Thanks @mrossinek for the comments and coding advice, they definitely contribute to improving my coding skills and my writing in english, once it is not my native language.

Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Victor, looks great. Thanks for continuing with this. I have one minor comment to the text. Can we change the intro a little - talking about noisy quantum computers is fine but we prefer to refrain from using NISQ to describe things. So maybe the following - and feel free to do something different if it does not work for you. I tried to preserve what I took from the meaning of what you had. The first part below is changed text, the part after continues with your original text as the latter part of the paragraph (i.e. the text starting with However, there are some problems... . I kept it all here so it can be seen/read at once but still distinguish what I changed


At the current stage of quantum computing the quantum processors we have available do not yet have error correcting codes implemented and thus still have noise that must be contended with. This noise can adversely impact the results and hence can limit the sort of problems we can currently tackle when, due to the noise, a high level of confidence and precision cannot
be expected from the results.

However, there are some problems which demand only few qubits to be solved, like the computation of the binding energy between the proton and neutron in the Deuteron nucleus [1]. This problem is an example of a scenario where we can achieve good results despite the challenges mentioned earlier.

@jvscursulim
Copy link
Contributor Author

Hi Victor, looks great. Thanks for continuing with this. I have one minor comment to the text. Can we change the intro a little - talking about noisy quantum computers is fine but we prefer to refrain from using NISQ to describe things. So maybe the following - and feel free to do something different if it does not work for you. I tried to preserve what I took from the meaning of what you had. The first part below is changed text, the part after continues with your original text as the latter part of the paragraph (i.e. the text starting with However, there are some problems... . I kept it all here so it can be seen/read at once but still distinguish what I changed

At the current stage of quantum computing the quantum processors we have available do not yet have error correcting codes implemented and thus still have noise that must be contended with. This noise can adversely impact the results and hence can limit the sort of problems we can currently tackle when, due to the noise, a high level of confidence and precision cannot be expected from the results.

However, there are some problems which demand only few qubits to be solved, like the computation of the binding energy between the proton and neutron in the Deuteron nucleus [1]. This problem is an example of a scenario where we can achieve good results despite the challenges mentioned earlier.

Hi Steve, thanks for the comment. I read your suggestion and I agree with the changes. I'll update the tutorial text.

Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Victor, thanks for your perseverance and work in contributing this notebook to Nature. I am very happy to be able to approve it - congrats on your contribution to Qiskit!

@jvscursulim
Copy link
Contributor Author

Victor, thanks for your perseverance and work in contributing this notebook to Nature. I am very happy to be able to approve it - congrats on your contribution to Qiskit!

Thanks Steve, I'm happy with the acceptance of this work as a contribution to Qiskit. I hope that I can make more contributions to Qiskit in the future.

@mergify mergify bot merged commit 5cc8eaa into qiskit-community:main Aug 12, 2022
Anthony-Gandon pushed a commit to Anthony-Gandon/qiskit-nature that referenced this pull request May 25, 2023
* Uploading the jupyter notebook that contains the tutorial about deuteron binding energy

* Uploading the jupyter notebook reformatted with black and the pydictlint file with some new techinal words.

* I fixed some typos in the jupyter notebook and added the 18 misspelled words pointed out by the spell check in the pydictlint file.

* Fixing the 'braket' typo in the jupyter notebook and deleting 'braket' of the pydictlint file.

* Fixing the problems in the .pylintdict

* Uploading an updated version of the tutorial with the changes requested by @mrossinek

* lint

* Adding acknowledgement in the .pylintdict

* Updating imports to the new modules location (second_q)

* Removing hardware and information exceptions

* Making the changes requested by @mrossinek

* lint

* Changing plots

* Including ansatze and performant

* Making some changes in the introduction

Co-authored-by: Max Rossmannek <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Community PR PRs from contributors that are not 'members' of the Qiskit organization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a new tutorial: deuteron binding energy
6 participants