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

Update mGGA implementation to new API and add unrestricted case #84

Merged
merged 13 commits into from
Nov 20, 2023

Conversation

dmejiar
Copy link
Contributor

@dmejiar dmejiar commented Nov 11, 2023

@wavefunction91 This adds restricted and unrestricted tau-mGGAs. The code for Laplacian mGGAs is also there but it is deactivated for now. I marked it as WIP as I did not have time to test the code.

@dmejiar dmejiar changed the title [WIP]: Update mGGA implementation to new API and add unrestricted case Update mGGA implementation to new API and add unrestricted case Nov 12, 2023
@dmejiar
Copy link
Contributor Author

dmejiar commented Nov 12, 2023

@wavefunction91 I have tested the restricted and unrestricted VXC and everything looks good.

I have not tested gradients as our code is not yet ready for that. We can comment that out, your call.

I activated the Laplacian contribution, which is always computed for simplicity. This poses a significant slowdown for tau-MGGAs as one ends up manipulating lots of zeros, but the logic to disable that contribution is already there, we just need to query the functional (which I think has to be through ExchCXX).

@wavefunction91
Copy link
Owner

@dmejiar I added an issue in ExchCXX to track the laplacian check
wavefunction91/ExchCXX#34

Copy link
Owner

@wavefunction91 wavefunction91 left a comment

Choose a reason for hiding this comment

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

Before checking this in, we'll need to add some unit tests. We can intercept ExaChem data directly if that's helpful. I have an example of how to write a fully compliant reference file here:

if( input.containsData("GAUXC.OUTFILE") ) {
// Create File
auto outfname = input.getData<std::string>("GAUXC.OUTFILE");
{ HighFive::File( outfname, HighFive::File::Truncate ); }
// Write molecule
write_hdf5_record( mol, outfname, "/MOLECULE" );
// Write Basis
write_hdf5_record( basis, outfname, "/BASIS" );
// Write out matrices
HighFive::File file( outfname, HighFive::File::ReadWrite );
HighFive::DataSpace mat_space( basis.nbf(), basis.nbf() );
HighFive::DataSpace sca_space( 1 );
auto dset = file.createDataSet<double>( "/DENSITY", mat_space );
dset.write_raw( P.data() );
if( integrate_vxc ) {
dset = file.createDataSet<double>( "/VXC", mat_space );
dset.write_raw( VXC.data() );
dset = file.createDataSet<double>( "/EXC", sca_space );
dset.write_raw( &EXC );
}
if( integrate_exx ) {
dset = file.createDataSet<double>( "/K", mat_space );
dset.write_raw( K.data() );
}
if( integrate_exc_grad ) {
HighFive::DataSpace grad_space( mol.size(), 3 );
dset = file.createDataSet<double>( "/EXC_GRAD", grad_space );
dset.write_raw( EXC_GRAD.data() );
}
}
}

Essentially - we need the basis, density, molecule, and EXC/VXC written out in that standard HDF5 file.

Also, we're going to need to add a check in the GPU code to preclude mGGAs for the time being. I'll add the implementation soon after this is merged.

@dmejiar
Copy link
Contributor Author

dmejiar commented Nov 14, 2023

Essentially - we need the basis, density, molecule, and EXC/VXC written out in that standard HDF5 file.

I have generated such files, but to enable the tests we need to either use the LibXC backend or have built-in implementations of mGGA functionals (tau and lapl) in ExchCXX.

Also, we're going to need to add a check in the GPU code to preclude mGGAs for the time being. I'll add the implementation soon after this is merged.

I added an early check while setting up the integral factory, do you think this would work?

@wavefunction91
Copy link
Owner

wavefunction91 commented Nov 15, 2023

LibXC our builtin mGGA implementations

OK - can you make a quick list of the mGGA functionals you'd like to see proper infrastructure for? Also can you add these reference files to the tests/reference_data directory?

I added an early check

Yep, that works for now. Thanks!!

@wavefunction91
Copy link
Owner

@dmejiar Any info about how the reference files were generated? Bfn tolerances? Grids? Pruning?

@dmejiar
Copy link
Contributor Author

dmejiar commented Nov 18, 2023

@dmejiar Any info about how the reference files were generated? Bfn tolerances? Grids? Pruning?

Hopefully the names of the file convey some of that info. These were obtained with the UltraFineGrid, SSF partitioning, and Robust pruning. I am not sure at what bfn tolerances you refer (we don't modify them in ExaChem) so they must have been the default

@wavefunction91
Copy link
Owner

wavefunction91 commented Nov 18, 2023

@dmejiar Can you share how you construct the XC functional for SCAN? Do you just add MGGA_X_SCAN, or is it compounded with MGGA_C_SCAN?

Also, for the cytosine test cases, are the UKS different spin states? Or are they just RKS run though the UKS code? If the latter, I'd prefer an actual UKS test is added, the way the UKS code works is pretty uninteresting for RKS data.

@dmejiar
Copy link
Contributor Author

dmejiar commented Nov 18, 2023

MGGA_X_SCAN plus MGGA_C_SCAN for the tauMGGA cases and MGGA_X_R2SCANL plus MGGA_C_R2SCANL for the Laplacian MGGA cases. The UKS data was generated for the +1 species (doublet).

@wavefunction91
Copy link
Owner

Unit tests added, but one of the reference files is missing data

dbwy@shire:~/Documents/Development/GauXC/tests/ref_data$ h5dump -H cytosine_r2scanl_cc-pvdz_ufg_ssf_robust.hdf5 
HDF5 "cytosine_r2scanl_cc-pvdz_ufg_ssf_robust.hdf5" {
GROUP "/" {
   DATASET "BASIS" {
      DATATYPE  H5T_COMPOUND {
         H5T_STD_I32LE "NPRIM";
         H5T_STD_I32LE "L";
         H5T_STD_I32LE "PURE";
         H5T_ARRAY { [16] H5T_IEEE_F64LE } "ALPHA";
         H5T_ARRAY { [16] H5T_IEEE_F64LE } "COEFF";
         H5T_ARRAY { [3] H5T_IEEE_F64LE } "ORIGIN";
      }
      DATASPACE  SIMPLE { ( 63 ) / ( 63 ) }
   }
   DATASET "MOLECULE" {
      DATATYPE  H5T_COMPOUND {
         H5T_STD_I32LE "Atomic Number";
         H5T_IEEE_F64LE "X Coordinate";
         H5T_IEEE_F64LE "Y Coordinate";
         H5T_IEEE_F64LE "Z Coordinate";
      }
      DATASPACE  SIMPLE { ( 13 ) / ( 13 ) }
   }
}
}

I added the stubs for that unit test here. Update the file, uncomment the test and we're g2g.

@wavefunction91
Copy link
Owner

@dmejiar Also, I just confirmed that this compiles / precludes the right paths when devices are enabled (even though there's a divergence with master after #87, I checked the merge locally and everything works as expected, no conflicts)

@dmejiar
Copy link
Contributor Author

dmejiar commented Nov 20, 2023

@wavefunction91 file updated

Copy link
Owner

@wavefunction91 wavefunction91 left a comment

Choose a reason for hiding this comment

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

LGTM

@wavefunction91 wavefunction91 merged commit d4956d1 into wavefunction91:master Nov 20, 2023
12 checks passed
wavefunction91 added a commit that referenced this pull request Nov 20, 2023
Daniel Mejia-Rodriguez added the CPU implementation of the meta-GGA XC integrator in #84.
@wavefunction91 wavefunction91 mentioned this pull request Nov 20, 2023
2 tasks
wavefunction91 added a commit that referenced this pull request Nov 21, 2023
* Add @dmejiar to README Contributors

Daniel Mejia-Rodriguez added the CPU implementation of the meta-GGA XC integrator in #84.

* Update README.md
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