MRChemSoft / mrchem

MultiResolution Chemistry
GNU Lesser General Public License v3.0
27 stars 21 forks source link

cavity function used directly instead of projected first #458

Closed gitpeterwind closed 10 months ago

gitpeterwind commented 1 year ago

Avoids memory consuming function trees, by using directly the analytical function. Note: requires the latest version of MRCPP Gabriel will test/polish before merge into master

robertodr commented 1 year ago

Thanks Peter!

gitpeterwind commented 10 months ago

How was it that we force it to use the latest mrcpp?

robertodr commented 10 months ago

Note that the solvation tests fail now:

         49 - reaction_operator (Failed)
         60 - Li_solvent_effect (Failed)

The former is a unit test, the latter an integration test. For the latter it might be enough to regenerate the reference output:

60:     JSON['output']['properties']['scf_energy']['E_next']....................................................................PASSED
60:     JSON['output']['properties']['scf_energy']['E_el']: computed value (-7.09457290) does not match (-7.09458382) to atol=1e-06, rtol=1e-06 by difference (0.00001092).
60:     JSON['output']['properties']['scf_energy']['Er_tot']: computed value (-0.06409434) does not match (-0.06408885) to atol=1e-06, rtol=1e-06 by difference (-0.00000550).
60:     JSON['output']['properties']['scf_energy']['Er_el']: computed value (0.12807619) does not match (0.12806527) to atol=1e-06, rtol=1e-06 by difference (0.00001092).
60:     JSON['output']['properties']['scf_energy']['Er_nuc']: computed value (-0.19217053) does not match (-0.19215412) to atol=1e-06, rtol=1e-06 by difference (-0.00001641).
gitpeterwind commented 10 months ago

This looks great @gitpeterwind! You made a lot of simplifications that I've been also making in #456 😄 For which system(s) have you been testing the memory footprint reductions? And what is the before and after consumption?

I've left some comments, especially about argument passing. Please have a look.

Actually @Gabrielgerez did most of the work. I mostly provided the function to multiply directly the orbitals with a representable function. (PS: I am in Oslo, and busy this week)

Gabrielgerez commented 10 months ago

@robertodr @gitpeterwind Now I have fixed what was needed and updated the input parsing (since i removed a keyword) and tests. All tests should be passing now. Sorry for the wait as well, attempted a big refactor to try and shave off the last few gb and it took longer than expected, so i had to branch it off

codecov[bot] commented 10 months ago

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (70e9fbd) 69.58% compared to head (516cc21) 69.25%. Report is 20 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #458 +/- ## ========================================== - Coverage 69.58% 69.25% -0.34% ========================================== Files 180 179 -1 Lines 15027 14975 -52 ========================================== - Hits 10457 10371 -86 - Misses 4570 4604 +34 ``` | [Files](https://app.codecov.io/gh/MRChemSoft/mrchem/pull/458?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft) | Coverage Δ | | |---|---|---| | [src/driver.cpp](https://app.codecov.io/gh/MRChemSoft/mrchem/pull/458?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL2RyaXZlci5jcHA=) | `72.68% <100.00%> (-0.04%)` | :arrow_down: | | [src/initial\_guess/core.cpp](https://app.codecov.io/gh/MRChemSoft/mrchem/pull/458?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL2luaXRpYWxfZ3Vlc3MvY29yZS5jcHA=) | `99.21% <100.00%> (ø)` | | | [src/initial\_guess/cube.cpp](https://app.codecov.io/gh/MRChemSoft/mrchem/pull/458?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL2luaXRpYWxfZ3Vlc3MvY3ViZS5jcHA=) | `95.94% <100.00%> (ø)` | | | [src/initial\_guess/gto.cpp](https://app.codecov.io/gh/MRChemSoft/mrchem/pull/458?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL2luaXRpYWxfZ3Vlc3MvZ3RvLmNwcA==) | `100.00% <100.00%> (ø)` | | | [src/initial\_guess/sad.cpp](https://app.codecov.io/gh/MRChemSoft/mrchem/pull/458?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL2luaXRpYWxfZ3Vlc3Mvc2FkLmNwcA==) | `98.92% <100.00%> (-1.08%)` | :arrow_down: | | [src/qmoperators/two\_electron/FockBuilder.cpp](https://app.codecov.io/gh/MRChemSoft/mrchem/pull/458?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL3Ftb3BlcmF0b3JzL3R3b19lbGVjdHJvbi9Gb2NrQnVpbGRlci5jcHA=) | `96.96% <100.00%> (+0.04%)` | :arrow_up: | | [src/qmoperators/two\_electron/ReactionOperator.h](https://app.codecov.io/gh/MRChemSoft/mrchem/pull/458?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL3Ftb3BlcmF0b3JzL3R3b19lbGVjdHJvbi9SZWFjdGlvbk9wZXJhdG9yLmg=) | `85.71% <ø> (-3.18%)` | :arrow_down: | | [src/qmoperators/two\_electron/ReactionPotential.h](https://app.codecov.io/gh/MRChemSoft/mrchem/pull/458?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL3Ftb3BlcmF0b3JzL3R3b19lbGVjdHJvbi9SZWFjdGlvblBvdGVudGlhbC5o) | `66.66% <ø> (-13.34%)` | :arrow_down: | | [tests/solventeffect/reaction\_operator.cpp](https://app.codecov.io/gh/MRChemSoft/mrchem/pull/458?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-dGVzdHMvc29sdmVudGVmZmVjdC9yZWFjdGlvbl9vcGVyYXRvci5jcHA=) | `100.00% <100.00%> (ø)` | | | [src/environment/SCRF.cpp](https://app.codecov.io/gh/MRChemSoft/mrchem/pull/458?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL2Vudmlyb25tZW50L1NDUkYuY3Bw) | `95.18% <96.42%> (+1.96%)` | :arrow_up: | | ... and [2 more](https://app.codecov.io/gh/MRChemSoft/mrchem/pull/458?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft) | | ... and [11 files with indirect coverage changes](https://app.codecov.io/gh/MRChemSoft/mrchem/pull/458/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Gabrielgerez commented 10 months ago

LGTM but there's two comments @Gabrielgerez should chime in on before this can be merged. Also:

  • One curiosity: How are you measuring the memory consumption?
  • What are the final before-and-after ballpark numbers on how much memory is needed for a solvent calculation?

Im using the log file output from slurm in betzy an example for a HF water calculation before and after: before

Memory statistics, in GiB:
ID             Alloc   Usage
732516         968.0        
732516.batch   242.0    20.0
732516.orted   726.0    57.5

after:

Memory statistics, in GiB:
ID             Alloc   Usage
732519         968.0        
732519.batch   242.0     9.1
732519.orted   726.0    22.7

When testing we were using a PBE0 calculation, where the gain showed a bit more, but now I am doing some proper scaling calculations with different sizes.

Gabrielgerez commented 10 months ago

LGTM but there's two comments @Gabrielgerez should chime in on before this can be merged. Also:

  • One curiosity: How are you measuring the memory consumption?
  • What are the final before-and-after ballpark numbers on how much memory is needed for a solvent calculation?

For the memory necessary for a solvent calculation, again an example of HF water gas phase

Memory statistics, in GiB:
ID             Alloc   Usage
732514         968.0        
732514.batch   242.0     6.9
732514.orted   726.0    12.9

solvent (eps=2.0, default for everything else)

Memory statistics, in GiB:
ID             Alloc   Usage
732519         968.0        
732519.batch   242.0     9.1
732519.orted   726.0    22.7
robertodr commented 10 months ago

Looks very good! What is the difference between batch and orted rows?

Gabrielgerez commented 10 months ago

Looks very good! What is the difference between batch and orted rows?

I was hoping @gitpeterwind could answer, I am assuming one is the memory used by the main process while the other is the shared one. The machine docs do not have much documentation on this, that I could easily find

gitpeterwind commented 10 months ago

Looks very good! What is the difference between batch and orted rows?

"Batch" is for the master compute node, while "orted" (Open Runtime Environment Daemon) is for the sum of all others compute nodes. Your run on 4 compute nodes , each of them has 242 GB available (3*242=726).

robertodr commented 10 months ago

So the sum of the two is the estimate of the memory consumption, right? If yes, the cost of the solvent went from ~4x to ~1.6x the cost of the vacuum calculation. Impressive!

gitpeterwind commented 10 months ago

So the sum of the two is the estimate of the memory consumption, right?

Yes, but the most relevant is the memory usage of the most memory consuming compute-node (normally master): it does not help to have compute nodes that consume little memory, if one of the compute nodes needs more memory than available. (compute nodes can not use memory from other compute nodes)