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

General class for N body grid integrations #191

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

marco-2023
Copy link
Collaborator

Added module and class for generic N grid integrations. This class is the base framework and on top of it, some porcelain will be added for the coulomb-exchange integrals.

Copy link
Member

@PaulWAyers PaulWAyers left a comment

Choose a reason for hiding this comment

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

It looks good to me. I'd let @Ali-Tehrani or @FarnazH review it before merging, however.

Can you add a tutorial notebook showing how to integrate the k-electron distribution function from Hartree-Fock (a Slater determinant) with the QC-Devs packages? That notebook could be added in the Horton3 repository, which is where most notebooks that use multiple packages go.

@marco-2023
Copy link
Collaborator Author

Thanks, @PaulWAyers for your feedback. I will certainly make the tutorial. Before that, I plan to implement the porcelain needed for the class so that the obtained grid (used for representing the k-electron distribution function) can be used for calculating the coulomb type integrals seamlessly (without having undefined values for any of the points).

@PaulWAyers
Copy link
Member

At first I'd just compute normalization of the distribution functions. Then we can work on coulomb integrals in a second step.

@marco-2023
Copy link
Collaborator Author

Perfect, I will do so then. Thank you very much.

@FarnazH FarnazH added the enhancement New feature or request label Oct 26, 2023
@FarnazH FarnazH added the future feature New feature that is coming soon label Jan 3, 2024
@marco-2023
Copy link
Collaborator Author

@sanchezw17 The Basegrid should not be changed to accommodate the Ngrid class. I will work on an example and see if I can reproduce your problem.

The function to integrate now has few requirements. It has to be a
function of n variables where n is the number of grids, all input
parameters must have the same dimensions as the corresponding
grid points. Added two tests too.
@marco-2023
Copy link
Collaborator Author

@sanchezw17 I made the corrections to the ngrid and added some tests. Now It is possible to use grids of different dimensions for the same function. At some point an example notebook will be needed but for now, you can see the tests. Please let me know if you have any problem after the corrections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request future feature New feature that is coming soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants