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

Introducing Gaussian Mixture Models #692

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

petschge
Copy link

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!

@streeve streeve added the enhancement New feature or request label Sep 19, 2023
Copy link
Member

@streeve streeve left a 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)

core/src/impl/Cabana_Bessel.hpp Outdated Show resolved Hide resolved
core/src/impl/Cabana_Matrix2d.hpp Show resolved Hide resolved
core/src/Cabana_Hammersley.hpp Outdated Show resolved Hide resolved
template <typename CellSliceType, typename WeightSliceType,
typename PositionSliceType, typename VelocitySliceType,
typename GaussianType>
size_t initializeRandomParticles( CellSliceType& cell, WeightSliceType& macro,
Copy link
Member

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, ... )

@streeve
Copy link
Member

streeve commented Sep 19, 2023

@tyounkin do you see things here that would be useful (or things you would want to extend/modify)?

core/src/Cabana_Hammersley.hpp Outdated Show resolved Hide resolved
core/src/impl/Cabana_GaussianWeight.hpp Outdated Show resolved Hide resolved
core/src/impl/Cabana_Erfinv.hpp Outdated Show resolved Hide resolved
};

// Define an execution policy
Cabana::SimdPolicy<cell.vector_length, Kokkos::DefaultExecutionSpace>
Copy link
Member

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

Copy link
Author

@petschge petschge Sep 23, 2023

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?

Copy link
Member

Choose a reason for hiding this comment

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

Here's one:

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

core/src/Cabana_ParticleInit.hpp Outdated Show resolved Hide resolved
core/src/Cabana_GaussianMixtureModel.hpp Outdated Show resolved Hide resolved
core/src/Cabana_GaussianMixtureModel.hpp Show resolved Hide resolved
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)
Copy link
Member

@streeve streeve left a 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

core/src/Cabana_GaussianMixtureModel.hpp Outdated Show resolved Hide resolved
};

// Define an execution policy
Cabana::SimdPolicy<cell.vector_length, Kokkos::DefaultExecutionSpace>
Copy link
Member

Choose a reason for hiding this comment

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

Here's one:

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

and put the implemenation in the GMMImpl class and the user facing bits
into one header file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants