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

Improve performance of computing molecular integrals and Hamiltonian #2316

Merged
merged 17 commits into from Mar 23, 2022

Conversation

soranjh
Copy link
Contributor

@soranjh soranjh commented Mar 14, 2022

Context:
The PR improves the performance of computing the electron repulsion integral and simplifying the qubit Hamiltonian.

Description of the Change:

  • The electron_repulsion function contained repeated calls inside nested for loops. These steps are now taken out of the oops and are computed only once.
  • The _hermite_coulomb function also contained a loop for computing the Boys function which is now removed and the Boys function is called once for evaluating an array. The _boys function is modified to adopt with this change.
  • The simplify function has more efficient ways for accessing the Hamiltonian terms..

Benefits:
The efficiency of building a molecular Hamiltonian is improved significantly.

Possible Drawbacks:
NA

Related GitHub Issues:
NA

@soranjh soranjh added WIP 🚧 Work-in-progress qchem ⚛️ Related to the QChem package labels Mar 14, 2022
@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@codecov
Copy link

codecov bot commented Mar 14, 2022

Codecov Report

Merging #2316 (b2ca3fe) into master (75e4c40) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2316   +/-   ##
=======================================
  Coverage   99.42%   99.42%           
=======================================
  Files         243      243           
  Lines       18322    18324    +2     
=======================================
+ Hits        18216    18218    +2     
  Misses        106      106           
Impacted Files Coverage Δ
pennylane/hf/hamiltonian.py 100.00% <100.00%> (ø)
pennylane/hf/integrals.py 100.00% <100.00%> (ø)
qchem/pennylane_qchem/qchem/hf/integrals.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 75e4c40...b2ca3fe. Read the comment docs.

@soranjh soranjh changed the title [WIP] Improve qchem workflow performance Improve performance of computing molecular integrals and Hamiltonian Mar 18, 2022
@soranjh soranjh removed the WIP 🚧 Work-in-progress label Mar 18, 2022
@soranjh soranjh marked this pull request as ready for review March 18, 2022 14:52
pennylane/hf/integrals.py Outdated Show resolved Hide resolved
Copy link
Contributor

@Jaybsoni Jaybsoni left a comment

Choose a reason for hiding this comment

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

Pretty straight forward, just a few clarifications.

Thanks,

qchem/pennylane_qchem/qchem/hf/integrals.py Show resolved Hide resolved
qchem/tests/hf/test_integrals.py Outdated Show resolved Hide resolved
qchem/pennylane_qchem/qchem/hf/integrals.py Outdated Show resolved Hide resolved
Copy link
Contributor

@obliviateandsurrender obliviateandsurrender left a comment

Choose a reason for hiding this comment

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

Looks good @soranjh!

@soranjh soranjh requested a review from Jaybsoni March 22, 2022 14:41
Copy link
Contributor

@Jaybsoni Jaybsoni left a comment

Choose a reason for hiding this comment

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

Looks good to go

Copy link
Contributor

@ixfoduap ixfoduap left a comment

Choose a reason for hiding this comment

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

Just double-check if using t == 0 or t == 0.0 is better. Otherwise good to go! Beautiful PR identifying a simple and very impactful change

pennylane/hf/hamiltonian.py Show resolved Hide resolved
pennylane/hf/integrals.py Show resolved Hide resolved
pennylane/hf/integrals.py Show resolved Hide resolved
@soranjh soranjh merged commit 91bcb5f into master Mar 23, 2022
@soranjh soranjh deleted the qchem_hf_performance branch March 23, 2022 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
qchem ⚛️ Related to the QChem package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants