Skip to content

LDA gradient comments #2839

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

Jump to bottom
Merged
JonathonMisiewicz merged 2 commits into from
Jan 10, 2023
Merged

LDA gradient comments #2839

JonathonMisiewicz merged 2 commits into from
Jan 10, 2023

Conversation

@JonathonMisiewicz
Copy link
Contributor

@JonathonMisiewicz JonathonMisiewicz commented Dec 21, 2022 •
edited
Loading

Obligatory @susilehtola and @hokru pings.

User API & Changelog headlines

  • Improved UKS gradient parallelization

Dev notes & details

  • Commented LSDA gradients
  • Synchronizing of RKS and UKS gradient code is deferred to a separate PR.

Status

  • Ready for review
  • Ready for merge

@JonathonMisiewicz JonathonMisiewicz added cleanup For issues where the goal is to make Psi4 a little cleaner. dft For issues specific to DFT and their many functionals. labels Dec 21, 2022
@JonathonMisiewicz JonathonMisiewicz added this to the Psi4 1.8 milestone Dec 21, 2022
loriab
loriab approved these changes Jan 4, 2023
View reviewed changes
Copy link
Member

@loriab loriab left a comment

Choose a reason for hiding this comment

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

Not the DFT reviewer you're looking for but lgtm

@JonathonMisiewicz
Copy link
Contributor Author

JonathonMisiewicz commented Jan 5, 2023

Susi is still on vacation, so I'll delay merging until either he approves or Wednesday.

susilehtola
susilehtola reviewed Jan 10, 2023
View reviewed changes
susilehtola
susilehtola reviewed Jan 10, 2023
View reviewed changes
susilehtola
susilehtola approved these changes Jan 10, 2023
View reviewed changes
Copy link
Member

@susilehtola susilehtola left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Can't see anything obviously wrong in the code, other than the two comments above. There are quite a lot of trivial changes i.e. just changing the initialization from foo to auto, which have no functional effects. The most important bit appears to just be that an OpenMP parallellization statement has been added.

@JonathonMisiewicz JonathonMisiewicz merged commit 78cfa2e into psi4:master Jan 10, 2023
@JonathonMisiewicz JonathonMisiewicz deleted the lda_gradient_commenting branch January 10, 2023 20:12
@JonathonMisiewicz JonathonMisiewicz mentioned this pull request May 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@susilehtola susilehtola susilehtola approved these changes

@hokru hokru hokru approved these changes

@loriab loriab loriab approved these changes

Assignees

No one assigned

Labels

cleanup For issues where the goal is to make Psi4 a little cleaner. dft For issues specific to DFT and their many functionals.

Projects

None yet

Milestone

Psi4 1.8

Development

Successfully merging this pull request may close these issues.

4 participants

@JonathonMisiewicz @susilehtola @hokru @loriab