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

Remaining tasks for AoS to SoA transition #861

Closed
7 tasks done
ye-luo opened this issue May 11, 2018 · 24 comments
Closed
7 tasks done

Remaining tasks for AoS to SoA transition #861

ye-luo opened this issue May 11, 2018 · 24 comments
Assignees
Labels
cleanup For cleanling legacy lines of code ECP help wanted

Comments

@ye-luo
Copy link
Contributor

ye-luo commented May 11, 2018

This is the final list of tasks that have priority before the remaining AoS code is fully removed from the source. The AoS removal likely will occur sometime in Oct. 2019. Even code listed as "priority" below will be removed once the deadline is exceeded if the assignee(s) have not ported by then and this issue will be closed.

All remaining AoS code has been encapsulated by if-defs. The changes made in the following PR highlight all remaining AoS code:

https://github.com/QMCPACK/qmcpack/pull/1846/files

Priority code to port before the Oct. deadline:

Code that is slated for deletion after the Oct. deadline (volunteers welcome!):

Note: tests that pass for AoS but fail for SoA:

  • long-diamondC_1x1x1_pp-vmc_sdj-1-16-flux (wavefunction error!)
  • short-c_no-hf_vmc-1-16 (numerical orbitals)
  • long-c_no-hf_vmc-1-16
  • short-c_no-sj_dmc-1-16
  • long-c_no-sj_dmc-1-16
  • short-H2-FDLR-16-1 (FDLR wavefunction)
  • short-H2-orb-opt-16-1 (orbital optimization)
  • short-H4-orb-opt-dmc-1-16
@ye-luo ye-luo added ECP cleanup For cleanling legacy lines of code labels May 11, 2018
@PDoakORNL
Copy link
Contributor

What about the optimizable wave function support?

I'm working on the CUDA code now.
I've begun JastrowCUDAClass switch to the OrbitalBase base class.

@ye-luo
Copy link
Contributor Author

ye-luo commented Jun 26, 2018

JastrowCUDAClass switch to the OrbitalBase base class what is this?
Currently, the code does

class OneBodyJastrowOrbitalBspline :
  public OneBodyJastrowOrbital<BsplineFunctor<OrbitalBase::RealType> >

OneBodyJastrowOrbital needs to be replaced with J1OrbitalSoA.

Jastrow optimization is supported by the CUDA build.

@PDoakORNL
Copy link
Contributor

The optimization I was talking about was of the single particle orbitals.

At the moment I have a J1OrbitalCUDASoA so as to look at what can be shared and what can't.
Not sure if it will be
J1OrbitalCUDASoA : public J1OrbitalSoA and eventually J1OrbitalCUDA : public J1Orbital
or
J1OrbitalCUDA : public J1OrbitalBase
J1OrbitalCPU : public J1OrbitalBase

So that we end up with a call like this:

return createOneBodyJastrow<J1OrbitalCUDASoA<RadFuncType>,DiffOneBodySpinJastrowOrbital<RadFuncType> >(cur);

A certain amount of the old CUDA OneBody and TwoBody code will go in but my current strategy is to make sure that any feature we don't have in CUDA we are following the SoA "method" and can degrade to the CPU implementation.

Although this is speculative at the moment since I'm still working in the determinant code.

@prckent
Copy link
Contributor

prckent commented Jun 28, 2018

Quick comment on the CUDA-> SoA transition, agreeing with @ye-luo 's list at the top: happily the CUDA SoA build is working for solids apart from runs involving Jastrows. e.g. https://cdash.qmcpack.org/CDash/testDetails.php?test=2572921&build=24790 shows that DMC is working with single determinant no Jastrow. After that the gaps are things like J3 and the new hybrid rep.

@PDoakORNL
Copy link
Contributor

RPAJastro is dependent on TwoBodyJastroOrbital. It also looks a bit unfinished in general.

@jtkrogel
Copy link
Contributor

jtkrogel commented Aug 2, 2018

@ye-luo, please add this to the checklist at the top: "evaluateHessian for J1 and J2". These are present in AoS, but not SoA and I am using them in research now.

@jtkrogel
Copy link
Contributor

@ye-luo please also add the evaluate_sp functions that appear in a number of QMCHamiltonianBase. These functions are needed to obtain and transmit single particle energies that are used by Traces, energy density, and energy density matrix.

@jtkrogel
Copy link
Contributor

Please also add Stress, see #970.

@prckent
Copy link
Contributor

prckent commented Mar 15, 2019

Note: We intend to flip the default to SOA shortly, since nearly all common features are implemented and we have observed that some users are overlooking the performance benefits of the SOA code. If anything critical is missing, please update this issue and the checklist at the top.

Current gating functionalities are the derivatives for backflow.

@prckent
Copy link
Contributor

prckent commented Jun 2, 2019

Update: SoA is now the default, but this issue remains relevant: at some point the AoS code will simply be deleted along with the legacy CUDA code. This will restore QMCPACK to having a single implementation "fork". To access functionality not ported will then require running an old version of QMCPACK or first porting the functionality to the current version.

@jtkrogel jtkrogel assigned markdewing and unassigned ye-luo and rcclay Aug 14, 2019
@jtkrogel
Copy link
Contributor

@spinedaflores Please can you comment here about your timeline for porting the orbital optimization code over?

@Paul-St-Young Is the lattice deviation estimator important to you? If so, can you do the port to SoA?

@jtkrogel jtkrogel changed the title Full list of known gaps and tasks for AoS to SoA transition Remaining tasks for AoS to SoA transition Aug 14, 2019
@spinedaflores
Copy link
Contributor

I'll aim to have my pull request ready by next Friday, but hopefully will have it done before then.

@jtkrogel
Copy link
Contributor

@spinedaflores Thanks. You are definitely on track to beat the rest of us.

@Paul-St-Young
Copy link
Contributor

@jtkrogel yes, I would like to keep the lattice deviation estimator working. I will look into porting

@markdewing
Copy link
Contributor

I'm thinking about putting #ifndef ENABLE_SOA around the distance table elements that are only for AoS (M, J, df, rinv, etc.) and around blocks of code that use these members. This would make the AoS sections of code more visible, and clearer when it is time to delete them.

@jtkrogel
Copy link
Contributor

I think this is a good idea.

@prckent
Copy link
Contributor

prckent commented Oct 23, 2019

Chiesa/Ceperley and bare force estimators are currently marked as done, but #2025 indicates they are not. Should we consider this task done or not?

@jtkrogel
Copy link
Contributor

Marked as done following discussion with @tiihonej. @tiihonej and/or @rcclay should comment here.

@tiihonej
Copy link
Contributor

@jtkrogel Yes, Chiesa/Ceperley and bare force estimators have been ported to SoA. @prckent, I thought #2025 refers to a different estimator with whose details I'm not familiar.

@prckent
Copy link
Contributor

prckent commented Nov 6, 2019

Calculation of pp derivatives for optimization needs to be captured above. It is a critical capability.

@jtkrogel
Copy link
Contributor

jtkrogel commented Nov 6, 2019

@prckent Issues have been added/updated.

@prckent
Copy link
Contributor

prckent commented Jan 6, 2020

All the items on the task list are now completed. #2179 was the last item. Following the v3.9.0 release, removal of the AoS code will commence.

@prckent prckent unpinned this issue Jan 7, 2020
@jtkrogel
Copy link
Contributor

Basically done?

@prckent
Copy link
Contributor

prckent commented Mar 24, 2021

Agreed. AoS is no longer a build option.

@prckent prckent closed this as completed Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup For cleanling legacy lines of code ECP help wanted
Projects
None yet
Development

No branches or pull requests

9 participants