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

AtomicOrbitalBasis and BackflowPooling #18

Closed
wants to merge 10 commits into from
Closed

AtomicOrbitalBasis and BackflowPooling #18

wants to merge 10 commits into from

Conversation

CheukHinHoJerry
Copy link
Collaborator

This branch implements the construction of the A basis for bflow wf.
@cortner It would be great if you can confirm this is what we need?

This code is not yet fully optimized and there is a lot to be done to make it more performant.

Also, currently the ProductBasis is a specific case and we should just implements a general version in Polynomials4ML. I will do that after confirming this is what we want.

CC @DexuanZhou

@cortner
Copy link
Member

cortner commented May 8, 2023

Thank you! This does look correct but there are so many changes it is hard to read. Can you maybe write a brief summary of how you re-organized the code and I'll judge from that?

@CheukHinHoJerry
Copy link
Collaborator Author

CheukHinHoJerry commented May 8, 2023

@cortner Sure. The original AtomicOrbitaBasis output the A basis directly. @DexuanZhou and I have made the following changes :

Implements the ProductBasis with SparseProduct:
The product basis is a mapping which maps x to the product of Rnl and Ylm with a given spec1.

Modify the AtomicOrbitalBasis as a wrapper of the ProductBasis:
It takes in x and apply different translation depends on nuclei defined during construction. And then it uses ProductBasis to evaluate the product on different translated x. The output of AtomicOrbitalBasis is a tensor of $\phi_{nlm}$ of different translations.

Move the BackFlowPooling out of
AtomicOrbitalRadius:
The pooling operation is moved out as a separate structure from the original AtomicOrbitalBasis. Which takes the tensor of $\phi_{nlm}$ from AtomicOrbitalBasis to the pooled A basis.

After implementing the above, I assmeble all the things by calling ProductBasis inside AtomicOrbitalBasis, and then following by a separate Pooling layer to construct the A basis.

@cortner
Copy link
Member

cortner commented May 15, 2023

I'll try to review this asap

@CheukHinHoJerry
Copy link
Collaborator Author

Maybe I can clean this up once first and give a summary before your review? I will do this within next 12 hours.

@cortner
Copy link
Member

cortner commented May 15, 2023

good, because I can't do it today or tomorrow :)

@CheukHinHoJerry
Copy link
Collaborator Author

This is moved to

#20 (comment)

I think we can delete this branch and also the PR.

CC @cortner @DexuanZhou

@CheukHinHoJerry
Copy link
Collaborator Author

Close and reference to #22

@DexuanZhou DexuanZhou deleted the 3d-2 branch January 9, 2024 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants