-
Notifications
You must be signed in to change notification settings - Fork 50
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
Introducing Gaussian Mixture Models #692
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a useful and relatively complete capability. I tried to keep comments relatively high level for now. This will need clang-format, doxygen comments, conflicts with ParticleInit fixed, and at least one unit test (it's not clear to me yet how to use this in the end)
template <typename CellSliceType, typename WeightSliceType, | ||
typename PositionSliceType, typename VelocitySliceType, | ||
typename GaussianType> | ||
size_t initializeRandomParticles( CellSliceType& cell, WeightSliceType& macro, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the conflicts with master are related to the rewrite of particle init. The changes were primarily to match the structured-grid side of the library, where each free function is now createParticles( CreationTypeTag, ... )
: currently only InitRandom
available, but this seems like it should become createParticles( InitGaussian, ... )
@tyounkin do you see things here that would be useful (or things you would want to extend/modify)? |
}; | ||
|
||
// Define an execution policy | ||
Cabana::SimdPolicy<cell.vector_length, Kokkos::DefaultExecutionSpace> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One other recent change across the library was to expose user-chosen execution spaces in addition to overloads with the default space for the memory being used - we'll want to support any valid exec_space here and then add a wrapper using the default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you point me to an example where you expose the capability to run on any execution space, but have a default selection for the user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's one:
Cabana/core/src/Cabana_VerletList.hpp
Line 728 in fa9e58a
void build( PositionSlice x, const std::size_t begin, const std::size_t end, |
We store the default exec space for a given memory space in the class to use in this overload, but also give the user the option to pass any (compatible) exec space in the next one
which is a number of weightes Gaussians that describe a particle distribution function in velocity space. The mixture model can be computed from a set of particles and we also allow to draw new particles from such a Gaussian Mixture Model. The code follows the papers by Figueiredo and Jain, 2002 (https://dx.doi.org/https://dl.acm.org/doi/10.1109/34.990138), but adds particles weights like the paper by Chen, Chacon, Nguyen, 2021 (DOI:https://doi.org/10.1016/j.jcp.2021.110185)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you'll need to rebase on master (or remove the last commit and merge master) to fix conflicts & remove these unrelated changes
}; | ||
|
||
// Define an execution policy | ||
Cabana::SimdPolicy<cell.vector_length, Kokkos::DefaultExecutionSpace> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's one:
Cabana/core/src/Cabana_VerletList.hpp
Line 728 in fa9e58a
void build( PositionSlice x, const std::size_t begin, const std::size_t end, |
We store the default exec space for a given memory space in the class to use in this overload, but also give the user the option to pass any (compatible) exec space in the next one
090aa55
to
6c20d05
Compare
and put the implemenation in the GMMImpl class and the user facing bits into one header file.
which is a number of weightes Gaussians that describe a particle distribution function in velocity space. The mixture model can be computed from a set of particles and we also allow to draw new particles from such a Gaussian Mixture Model. The code follows the papers by Figueiredo and Jain, 2002
(https://dx.doi.org/https://dl.acm.org/doi/10.1109/34.990138), but adds particles weights like the paper by Chen, Chacon, Nguyen, 2021 (DOI:https://doi.org/10.1016/j.jcp.2021.110185)
This is not against the current head of master, but I assume that there is other changes that I need to make before rebasing and submitting for good. Feedback (here or on Slack) welcome!