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

Put spline grid and coefficient information into MultiBspline #148

Open
markdewing opened this issue Jul 13, 2018 · 2 comments
Open

Put spline grid and coefficient information into MultiBspline #148

markdewing opened this issue Jul 13, 2018 · 2 comments

Comments

@markdewing
Copy link
Collaborator

It feels like there are too many layers to the spline code (especially the allocation). I think this is a result of the impedance mismatch between C and C++, and gradually moving the code from its original C to C++. It would be nice to move splines to a full C++ class.

Current the MultiBspline class takes the spline data struct as argument to the various functions. (Using it as a C-style object). MultiBspline has no data of it's own. Would it work to move the grid and coefficient data as member data of the MultiBspline class? Is there some advantage to keeping it as a light class with no data?
How would this change affect Kokkos?

@ye-luo
Copy link
Collaborator

ye-luo commented Jul 13, 2018

There are three layers: SPO(C++), Spline evaluation(C++) and data(C).
the number of SPO and the number of splines can be different depending on symmetry. So SPO (physics) calls the spline evaluation(math) and applies the phase and symmetry. So these two layers can not be merged.
The splitting of the spline evaluation and data is on purpose because the data is shared via pointer among threads. SPO provides the result vector and scratch memory for spline evaluation and so it is thread-private. I also don't like the C-style object. I have limited idea whether to fuse or how to fuse but the requirement needs to be met.

@lshulen
Copy link

lshulen commented Jul 13, 2018

Ray has some notes in https://github.com/rcclay/miniqmc/wiki . The C style structs do cause a problem for kokkos (although one that can be solved very easily). I would be in favor of updating them, and in principle think it makes sense to have them inside MultiBspline, but Ye's point that separating data and evaluation makes physical sense is also important.

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

3 participants