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.

Sign up for GitHub

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
+16 −4
Open

Fix for a bug inside of christiansen_integrals_dipole #6870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6dfb9d4
small fix
austingmhuang Jan 21, 2025
c41f04a
Apply suggestions from code review
austingmhuang Jan 21, 2025
740b80d
Merge branch 'master' into bugfix_dipole
austingmhuang Jan 21, 2025
3a0055a
this would fail
austingmhuang Jan 21, 2025
fd72be1
this would fail
austingmhuang Jan 21, 2025
5de30e5
changelog and black
austingmhuang Jan 27, 2025
e5d5ddb
Merge branch 'master' into bugfix_dipole
austingmhuang Jan 27, 2025
5846d52
Merge branch 'master' into bugfix_dipole
austingmhuang Jan 30, 2025
4671401
fix
austingmhuang Jan 30, 2025
aeac90f
fix
austingmhuang Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/releases/changelog-dev.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,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.
Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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 🐱

Copy link
Contributor

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:

New features since last release

Labs 🧪

Improvements 🛠

[#6870](https://github.com/PennyLaneAI/pennylane/pull/6870)

<h3>Contributors ✍️</h3>

This release contains contributions from (in alphabetical order):
Expand All @@ -165,6 +168,7 @@ Isaac De Vlugt,
Diksha Dhawan,
Pietropaolo Frisoni,
Marcus Gisslén,
Austin Huang,
Christina Lee,
Mudit Pandey,
Andrija Paurevic
8 changes: 8 additions & 0 deletions pennylane/labs/tests/vibrational/test_christiansen_ham.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ def test_christiansen_integrals_dipole():
assert np.allclose(abs(three), abs(D3), atol=1e-8)


def test_christiansen_integrals_dipole_conditional():
"""Test that christiansen_integrals_dipole works even when there is only two mode dipoles."""
coeffs = christiansen_integrals_dipole(pes=pes_object_2D, n_states=4)
assert np.allclose(abs(coeffs[0]), abs(D1), atol=1e-8)
assert np.allclose(abs(coeffs[1]), abs(D2), atol=1e-8)
assert len(coeffs) == 2


def test_cform_onemode():
"""Test that _cform_onemode produces the expected one-body integral."""
flattened_H1 = H1.ravel()
Expand Down
8 changes: 4 additions & 4 deletions pennylane/labs/vibrational/christiansen_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -847,8 +847,9 @@ def christiansen_integrals_dipole(pes, n_states=16):
path.unlink()
comm.Barrier()
dipole_cform_onebody = comm.bcast(dipole_cform_onebody, root=0)
D_arr = [dipole_cform_onebody, None, None]

if pes.localized is True or pes.dipole_level > 1:
if pes.dipole_level > 1:
austingmhuang marked this conversation as resolved.
Show resolved Hide resolved
local_dipole_cform_twobody = _cform_twomode_dipole(pes, n_states)
obliviateandsurrender marked this conversation as resolved.
Show resolved Hide resolved
comm.Barrier()

Expand All @@ -864,8 +865,9 @@ def christiansen_integrals_dipole(pes, n_states=16):
path.unlink()
comm.Barrier()
dipole_cform_twobody = comm.bcast(dipole_cform_twobody, root=0)
D_arr = [dipole_cform_onebody, dipole_cform_twobody, None]

if pes.localized is True or pes.dipole_level > 2:
if pes.dipole_level > 2:
local_dipole_cform_threebody = _cform_threemode_dipole(pes, n_states)
ddhawan11 marked this conversation as resolved.
Show resolved Hide resolved
comm.Barrier()

Expand All @@ -883,7 +885,5 @@ def christiansen_integrals_dipole(pes, n_states=16):
dipole_cform_threebody = comm.bcast(dipole_cform_threebody, root=0)

D_arr = [dipole_cform_onebody, dipole_cform_twobody, dipole_cform_threebody]
else:
D_arr = [dipole_cform_onebody, dipole_cform_twobody]

return D_arr
Loading