-
Notifications
You must be signed in to change notification settings - Fork 623
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
Fix for a bug inside of christiansen_integrals_dipole #6870
base: master
Are you sure you want to change the base?
Conversation
Hello. You may have forgotten to update the changelog!
|
Please add a test that catches the bug. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6870 +/- ##
==========================================
- Coverage 99.59% 99.59% -0.01%
==========================================
Files 478 478
Lines 45295 45293 -2
==========================================
- Hits 45113 45111 -2
Misses 182 182 ☔ View full report in Codecov by Sentry. |
@@ -142,6 +142,9 @@ | |||
* `BasisState` now casts its input to integers. | |||
[(#6844)](https://github.com/PennyLaneAI/pennylane/pull/6844) | |||
|
|||
* Fixed a bug in `qml.labs.vibrational.christiansen_integrals_dipole` that caused one-mode and two-mode dipole operators to be computed incorrectly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to go into the labs
changelog? Also maybe it is already known but the qml.labs.vibrational
is not accessible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There will be a PR to fix the labs vibrational docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me move it into the labs
changelog. I forgot that this was not in qchem
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @DSGuala where lab section 🐱
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's have the sections like this:
Context:
If there isn't a two/three-mode dipole, we shouldn't be computing the integral coefficients.
Description of the Change:
It should just check that the dipole_level is greater than the minimum needed. The localization is irrelevant.
Benefits:
Now you can compute one-mode/two-mode integral coefficients without a random error.
Possible Drawbacks:
None
Related Github Issues:
None