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

Integration test needed for the ACForce estimator #2238

Open
tiihonej opened this issue Jan 22, 2020 · 9 comments
Open

Integration test needed for the ACForce estimator #2238

tiihonej opened this issue Jan 22, 2020 · 9 comments
Assignees

Comments

@tiihonej
Copy link
Contributor

QMCPACK runs with the ACForce estimator crash at launch with the latest code (since around Jan 16). The error message is something like the following:

Resetting Properties of the walkers 1 x 50
ERROR You cannot resize a constant size matrix beyond its initial max dimensions
Fatal Error. Aborting at Unhandled Exception
*** The MPI_Abort() function was called after MPI_FINALIZE was invoked.
*** This is disallowed by the MPI standard.
*** Your MPI job will now abort.
[psi3:13270] Local abort after MPI_FINALIZE started completed successfully, but am not able to aggregate error messages, and not able to guarantee that all other processes were killed!

I take suggestions to better pinpoint where this is coming from. Sample inputs are attached:
vmc_force_jan2020.tar.gz

@jtkrogel jtkrogel added the bug label Jan 22, 2020
@jtkrogel
Copy link
Contributor

Sounds like a good target for addtional unit/deterministic/integration tests.

@PDoakORNL PDoakORNL self-assigned this Jan 22, 2020
@prckent
Copy link
Contributor

prckent commented Jan 22, 2020

Ack! A simple view of this would be that we need a simple integration test and preferably a deterministic integration test that uses ACForce. However, better would be a unit or small scale integration test that can catch this.

@PDoakORNL
Copy link
Contributor

#2232 will be done today and should fix this

@tiihonej
Copy link
Contributor Author

#2242 Did not fix it for me. I could write a simple integration test for ACForce, @prckent . Actually, the above inputs slightly modified would probably work?

@prckent
Copy link
Contributor

prckent commented Jan 24, 2020

@tiihonej Please modify an existing test, e.g. LiH dimer. This will avoid introducing new files for the wavefunctions and potentials. Of course, this assumes that LiH triggers the test. I suspect that parsing the force to check the numerical results will require writing some new python code in the test system, but for now that is not essential: the key test is whether the code crashes or not.

@tiihonej
Copy link
Contributor Author

@prckent Modifying existing file should be alright and, and LiH already covers a great deal of force-related features (the only exception probably being the derivatives of some higher-order WF-components). Testing against the values of the Pulay terms should be straightforward and catch all there is to catch regarding the actual performance.

@PDoakORNL
Copy link
Contributor

PDoakORNL commented Jan 24, 2020

@tiihonej Do you mean you get the same resize error?

I'm running your example with this command line

OMP_NUM_THREADS=1 mpirun --mca btl tcp,self -np 16 -map-by numa:PE=1 ../bin/qmcpack vmc.force.xml

@tiihonej
Copy link
Contributor Author

@PDoakORNL Nevermind that, there was something else wrong with my build. I did a fresh checkout and everything works. I'll push an integration test and then close this issue.

@tiihonej
Copy link
Contributor Author

There are a few issues with adding an integration test for the current ACForce estimator.

First, none of the needed observables (ACForce_{hf,pulay,wfgrad, Ewfgrad}) are implemented in the checking macros or nexus. Adding them is perhaps a bit tedious, considering that the final observable format is perhaps subject to discussion (ie, is scalar.dat a proper place for tensorial quantities? Should QMCPACK sum up the pulay corrections internally and only/also output the final force?).

Second, LiH_dimer_pp is a good and simple platform for the test, but the wavefunction is too good! At equilibrium we get essentially zero forces and corrections, which is not so good for testing.

I think a test like this should only be added once we are done with streamlining the code, including nexus support.

@tiihonej tiihonej changed the title Force estimator broken by recent commits Integration test needed for the ACForce estimator Jan 24, 2020
@ye-luo ye-luo added the force label Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants