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

Jastrow #98

Merged
merged 8 commits into from
Jan 4, 2023
Merged

Jastrow #98

merged 8 commits into from
Jan 4, 2023

Conversation

scemama
Copy link
Member

@scemama scemama commented Sep 6, 2022

No description provided.

@scemama scemama marked this pull request as draft September 6, 2022 15:45
@scemama scemama marked this pull request as ready for review January 2, 2023 15:19
@scemama scemama requested a review from q-posev January 3, 2023 10:19
@scemama
Copy link
Member Author

scemama commented Jan 3, 2023

I added the Jastrow group in the Correlation factors section.

There are many diffs because I reorganized the trex.org file as follows:

◉ Metadata (metadata group)...
◉ System
○ Nucleus (nucleus group)...
○ Cell (cell group)...
○ Periodic boundary calculations (pbc group)...
○ Numerical integration grid (grid group)...
○ Electron (electron group)...
○ Ground or excited states (state group)...
◉ One-electron basis
○ Basis set (basis group)...
○ Effective core potentials (ecp group)...
○ Atomic orbitals (ao group)...
○ Molecular orbitals (mo group)...
◉ N-electron basis
○ Slater determinants (determinant group)...
○ Configuration state functions (csf group)...
○ Amplitudes (amplitude group)...
○ Reduced density matrices (rdm group)...
◉ Correlation factors
○ Jastrow factor (jastrow group)...
◉ Quantum Monte Carlo data (qmc group)...

Copy link
Member

@q-posev q-posev left a comment

Choose a reason for hiding this comment

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

Thanks @scemama !

Have this been tested? I noticed that the new jastrow group has for example: ~een_num~ | ~dim~ | ~(nucleus.num)~. Normally this should not work since we do not support arrays of dim type. Especially since you later use this dim in the | ~een~ | ~float~ | ~(jastrow.een_num)~ , which value of jastrow.een_num will be used? We had the same issue with the non_local components in the ecp group and ended up re-structing the whole group to accommodate all options.

I also find the One-/N-electron basis reorganization slightly confusing.

@scemama
Copy link
Member Author

scemama commented Jan 3, 2023

You are right! I have changed een and een_num to make them depend on the number of nuclei, but it does not make sense now... I will fix that. Thanks!

@scemama
Copy link
Member Author

scemama commented Jan 3, 2023

OK, I fixed the dim problem and added some tests. Everything seems OK now.

I restructured like that:
◉ System
*○ Nucleus (nucleus group)...
*○ Cell (cell group)...
*○ Periodic boundary calculations (pbc group)...
*○ Electron (electron group)...
*○ Ground or excited states (state group)...
◉ Basis functions
*○ Basis set (basis group)...
*○ Effective core potentials (ecp group)...
*○ Numerical integration grid (grid group)...
◉ Orbitals
*○ Atomic orbitals (ao group)...
*○ Molecular orbitals (mo group)...
◉ Multi-determinant information
*○ Slater determinants (determinant group)...
*○ Configuration state functions (csf group)...
*○ Amplitudes (amplitude group)...
*○ Reduced density matrices (rdm group)...
◉ Correlation factors
*○ Jastrow factor (jastrow group)...
◉ Quantum Monte Carlo data (qmc group)...

Better? :-)

Copy link
Member

@q-posev q-posev left a comment

Choose a reason for hiding this comment

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

Thanks, Anthony! All good now, I am merging 👍

@q-posev q-posev merged commit bc5f70f into master Jan 4, 2023
@scemama scemama deleted the jastrow branch January 4, 2023 13:09
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.

2 participants