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

L2 DMC #1948

Merged
merged 37 commits into from
Nov 5, 2020
Merged

L2 DMC #1948

merged 37 commits into from
Nov 5, 2020

Conversation

jtkrogel
Copy link
Contributor

Putting up this WIP PR to gather feedback on the merge of new DMC projector features for L2 potentials into mainline. I'm particularly interested in any issues spotted relative to #1947.

Please ignore the DMCUpdatePbyPVMC and DMCUpdatePbyPL2VMC classes. These are development scaffolding and will be removed. The main task is to merge DMCUpdatePbyPL2 and DMCUpdatePbyPWithRejectionFast in a manner that is suitable for current mainline.

Other changes of note reside in DriftOperators and QMCHamiltonian.

@qmc-robot
Copy link

Can one of the admins verify this patch?

@markdewing
Copy link
Contributor

What's an L2 potential? Is the background published somewhere?

@jtkrogel
Copy link
Contributor Author

Sorry, this is a pseudo-Hamiltonian in the literature (but with the "a" term set to zero, keeping only the L^2, or "b", term).

See https://doi.org/10.1103/PhysRevLett.62.2088.

@PDoakORNL
Copy link
Contributor

PDoakORNL commented Sep 27, 2019

Main thing that I think is going to impact you is the Update layer is removed. advanceWalker is replaced by a static function advanceWalkers See VMCBatched::advanceWalkers, DMCBatched::advanceWalkers.

They're a number of ways we could to deal with specialized updates, since in your case you're only overiding advanceWalkers it suggests that choosing an implementation for advanceWalkers at construction of the driver is sufficient.

class DMCBatched
{
...
using AdvanceWalkersFunction = std::function<void(const DMCBatched::StateForThread&, Crowd&,DriverTimers&,ContextForStep&,bool)>;
DMCBatched(...,advanceWalkers) : ... advanceWalkers_(advanceWalkers) {}
...
AdvanceWalkersFunction advanceWalkers_;
...
}

class DMCL2Advance {
void operator()(const DMCBatched::StateForThread& sft, Crowd& crowd, ContextForSteps& move_context, bool recompute)
{
...
}

I think its clear much of the advanceWalkers_ code is actually shared, the specialized section is a subset of this, how consistent and how small will require an examination of different update codes we actually want to keep.

@jtkrogel
Copy link
Contributor Author

This branch is now up to date with develop. I'm going to add a deterministic test, clean up the code a bit, and then request a merge.

It will be good to have this functionality released prior to the move to batched drivers as it is a core technology for the Reboredo FWP. Having tests, etc, merged will also facilitate the transition to batched drivers when they are ready.

@jtkrogel jtkrogel changed the title [WIP] L2 DMC L2 DMC Oct 30, 2020
@jtkrogel
Copy link
Contributor Author

Ready for merge.

@ye-luo
Copy link
Contributor

ye-luo commented Oct 30, 2020

Okay to test

@prckent
Copy link
Contributor

prckent commented Nov 3, 2020

@jtkrogel What are you thoughts on the mixed precision builds that are failing in the CI? Hopefully they are small fixes.

@jtkrogel
Copy link
Contributor Author

jtkrogel commented Nov 4, 2020

Mixed precision build issues should be resolved now.

@ye-luo
Copy link
Contributor

ye-luo commented Nov 4, 2020

@jtkrogel could you resolved the reorder warning. Basically the declaration order is not consistent with the constructor initialization order.

Copy link
Contributor

@ye-luo ye-luo left a comment

Choose a reason for hiding this comment

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

I fixed the GCC reorder warning. Get this in before the upcoming release.

@ye-luo ye-luo merged commit 6b11c06 into QMCPACK:develop Nov 5, 2020
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.

7 participants