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

Backflow SoA Migration 2/2 #1225

Merged
merged 10 commits into from Dec 7, 2018

Conversation

Projects
None yet
4 participants
@rcclay
Copy link
Contributor

rcclay commented Dec 7, 2018

This PR fixes the AoS/SoA distance table discrepancy in the backflow transformation. This enables me to run VMC/DMC with/without drift within SoA or AoS, using backflow wavefunctions.

Some notes:
1.) This is a "reference" implementation. Only the interfaces to the distance tables were modified. No attempt has been made to change the algorithm.
2.) Currently, the code is split between AoS and SoA code paths with #ifdef ENABLE_SOA. While some cringe at macros, I figure the goal is to pull out AoS eventually, so this forked code path shouldn't be long lived.

@rcclay rcclay requested review from markdewing and ye-luo Dec 7, 2018

@wafflebot wafflebot bot added the in progress label Dec 7, 2018

@qmc-robot

This comment has been minimized.

Copy link
Collaborator

qmc-robot commented Dec 7, 2018

Can one of the maintainers verify this patch?

1 similar comment
@qmc-robot

This comment has been minimized.

Copy link
Collaborator

qmc-robot commented Dec 7, 2018

Can one of the maintainers verify this patch?

@prckent

This comment has been minimized.

Copy link
Contributor

prckent commented Dec 7, 2018

OK to test

@prckent

This comment has been minimized.

Copy link
Contributor

prckent commented Dec 7, 2018

Good use of ifdefs here. I agree the cleanup will be easier.

@ye-luo

This comment has been minimized.

Copy link
Contributor

ye-luo commented Dec 7, 2018

Does the BF optimization work with SoA or not?
Does BF works with complex? heg sjb is not added with the complex build.
Could you make a summary what is left using BF with the spline SPO?

Please add an issue that we don't have a test for the eI case.
I check the heg tests. They all pass with SoA now.

@ye-luo

ye-luo approved these changes Dec 7, 2018

@rcclay

This comment has been minimized.

Copy link
Contributor

rcclay commented Dec 7, 2018

@ye-luo
1.) BF optimization seems to work.
2.) BF works with complex. All development was done on the complex build with spot checks on the real build.
3.) For spline SPO, I think all the major functionality has been handled on the spline. For the BF transformations, I didn't implement all the various evaluate() calls present in the AoS version, but its not apparent to me how many of them are actually used. If an exotic estimator gets broken, I'll fix it.

Do you mean to add a test case for electron-ion backflow? Some of the bccH tests certainly use an e-I backflow term...

@ye-luo

This comment has been minimized.

Copy link
Contributor

ye-luo commented Dec 7, 2018

Yes. A test case for electro-ion backflow will be needed.
We also don't have any backflow tests with the complex. I tried to enable the heg which seems still still failing. They don't need to be resolved in this PR. Open an issue for a record and fix it later.

@ye-luo

This comment has been minimized.

Copy link
Contributor

ye-luo commented Dec 7, 2018

#1226 #1227 have been added to track needed tests.

@prckent

This comment has been minimized.

Copy link
Contributor

prckent commented Dec 7, 2018

Do we have a backflow optimization test?

@rcclay

This comment has been minimized.

Copy link
Contributor

rcclay commented Dec 7, 2018

@prckent We have a test of all derivatives of the wavefunction w.r.t. optimizable parameters needed for the linear method, but I don't think we have a test of a realistic optimization run yet.

@prckent

This comment has been minimized.

Copy link
Contributor

prckent commented Dec 7, 2018

Does the existing test imply that the "hand off" to the optimizer will be correct? If so, I think a bf-optimizer test is only nice to have and not critical. We will need a worked bf example at some point.

@rcclay

This comment has been minimized.

Copy link
Contributor

rcclay commented Dec 7, 2018

@prckent I would say yes, and I've verified that things do in fact optimize on the side. Someone should point me to an optimization test template, or we should chat about what's desired out of such a test for backflow. Checking to see if it doesn't crash is easy, but BF has always been fickle for optimization. So saying we should gain X mHa for energy and reduce the variance by Y% for a test pass seems a bit daunting.

@prckent

This comment has been minimized.

Copy link
Contributor

prckent commented Dec 7, 2018

The art of making an optimizer test is finding a small enough but still meaningful system that can be run fast and reliably enough (3sigma). Perhaps few electron HEG?

@prckent prckent merged commit 7183da7 into QMCPACK:develop Dec 7, 2018

2 checks passed

qmcpack rhea Build finished.
Details
qmcpack rhea (gpu) Build finished.
Details

@wafflebot wafflebot bot removed the in progress label Dec 7, 2018

@rcclay rcclay deleted the rcclay:bf_soa_2 branch Dec 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment