ENH Make GMM initialization more robust and easily user customizeable #24812
Conversation
Also remove unused code line in fit_predict. Not sure why this was left in from a previous refactor but it's not accessed.
get_responsibilities takes in array of incides or labels and gives the responsibilities. This logic was previously buried in the _initialize_parameters of the base class separately for each init_params. Now it will be easier for users to impute own responsibilities and provide a callable that returns responsibilities.
add function to check responsibilities to `_base` to be used for validating callable that returns responsibilities. Modify GMM to allow initializing a GMM based on user input responsibilities, with new `responsibilities_init` and use the new function to validate its input. Minor bugfixes to logic pre-testing.
According to tests, dtype should not be e.g. np.int32, but should be inexact numpy array. Removed explicit dtype to pass tests.
Also making sure we don't have future regressions using new feature by checking against old (manual) version of the responsibility creator. Add test for random_state initialization. Add tests for _check_responsibilities and get_responsibilities.
Previously, it would trigger during check_parameters which runs also on every fit operation, meaning that responsibilities could not be of different size to X. Also added a data permutation test to avoid regressions.
Co-authored-by: Chiara Marmo <cmarmo@users.noreply.github.com>
|
@cmarmo Thanks for the review! CI is passing now. |
|
Thanks @emirkmo for fixing the CI. I'm afraid I'm probably not the right person for a review here.
In my opinion this will speed up a bit the review process, but please be patient as 1.2 is in preparation and that means older PRs should probably go in first. |
@cmarmo Sorry that I have not responded. The issue is that both are actually the same thing. The responsibility matrix is the tunnel that everything goes through. The callable is just a way of creating it. Currently, this step is hard coded into the init methods passed via a string! So it is not testable or generalizable to a callable either. In fact, a unified API for passing in responsibilities is really the first step. Whether these are user defined or the API is used for the current string based init methods.. The callable part could come in later. It's just trivial to add once the responsibility API is done. Do you still suggest that I split it? The reverse of allowing a callable but not user defined responsibilities is very difficult, see below.It would be difficult and suboptimal to separate the two, especially the callable part by itself. Technically it is possible. But I would be adding 95% of the "allowing to pass user-defined 'responsibilites'.", but then working hard to not expose this to the user.Technically, it is possible to only add the callable, but like I said it would require some bad code practices and engineering to check/validate the responsibilities created and passed in by the callable while not exposing this to the user. It also would be very opaque how to define a proper callable. Currently we do not need to tell the user how to compute responsibilities, we have that hardcoded into the init method, but if we add the option to pass a callable that creates a responsibilities matrix, we would need to define and show how to create it, otherwise I don't see how to document the callable option:
|
|
Hi @emirkmo , thanks for your explanations: as I said I'm not the right person for a review here, so if you think this is the right direction for this PR , I'm afraid the only thing to do next is wait for a reviewer... hope they will come ASAP. |
Reference Issues/PRs
Fixes #23195. Fixes #24811.
What does this implement/fix? Explain your changes.
Allows passing in a custom callable to initialize
GaussianMixtureclass (and the parentBaseMixtureclass), as discussed inAdd more robust Kmeans initializer for mixture.GaussianMixture #23195 (also see comment Add more robust Kmeans initializer for mixture.GaussianMixture #23195 (comment)). Adds appropriate tests, input validation.
Additionally, allows easily passing in user defined
responsibilitiesto exactly initialize aGaussianMixtureclass (Allow directly passing in responsibilities to GMM #24811), with the newresponsibilities_initparameter. The publicget_responsibilitiesfunction is now used in the initialization of theBaseMixtureclass, and is tested properly.Docs are expanded to describe the new initialization. Tests are added. See the linked issues for more details.
Any other comments?
It has been difficult to robustly initialize GMM since the initialization uses hardcoded parameters and the user really only has control over the type of algorithm to use, but not on any specific initialization, or with more robust user-chosen parameters (which are needed when data is sparse or highly structured, for example). Although it was always possible to hack together private functions from the
_gaussian_mixture.pyto properly do this while using the_base.pyclass for reference, and using for exampleclusters.KMeansto do a fit first to use for initialization, this was difficult in practice.It required calculating means, weights, and precisions (after first reverse-engineering and calculating responsibilities), which are not trivial, nor explained. Since all of these are calculated from responsibilities initialized from the
init_paramsinside the GMM class anyway, this PR now adds a way to simple pass in the responsibilities instead, and adds a helper function to calculate them fromKMeans,kmeans_plusplus, or custom initializers, in a generic way.Also removed some unreachable code and increased test coverage, added tests to show that these changes produce the same behavior as the old initializer, harder to test, code, and added some tests which should act as regression tests or edge case tests. Finally, I am not sure what the best place for this
get_responsibilitieshelper function is, and whether it should be exposed in the__init__.pyfor the module.All suggestions welcome, as this is my first code contribution to scikit-learn. Black, Flake8, and MyPy were run successful on my local machine, and the docs compiled.