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

Incremental PCA #532

Merged
merged 18 commits into from Feb 11, 2015

Conversation

Projects
None yet
3 participants
@jalabort
Member

jalabort commented Jan 7, 2015

This PR adds the method increment to the PCAModel class.

It also:

  • Add ipca function in menpo.math.decomposition
  • Change name of function principal_component_decomposition to pca
  • Remove bias and whiten kwargs from pca function.

The code is fairly stable and seems to be working as expected. The code can be easily tested using the following two notebooks on my scrap repo: IPCA and IPCA - Menpo Model.

However, before this is merged in, I want to clean up the PCAModel class a bit since there are things that are not correct at the moment.

To do:

  • Add tests for ipca
  • Add tests for increment on PCAModel

jalabort added some commits Jan 7, 2015

Add increment method to PCAModel class.
- Add ipca funtion in menpo.math.decomposition
- Change name of function principal_component_decomposition to pca
- Remove bias and whiten kwargs from pca function.

@jabooth jabooth added the in progress label Jan 7, 2015

@jalabort jalabort self-assigned this Jan 7, 2015

jalabort added some commits Jan 7, 2015

Add increment method to PCAModel class.
- Add ipca funtion in menpo.math.decomposition
- Change name of function principal_component_decomposition to pca
- Remove bias and whiten kwargs from pca function.

Conflicts:
	docs/source/api/menpo/math/index.rst
	docs/source/api/menpo/math/principal_component_decomposition.rst
	docs/xref_map.py
@jalabort

This comment has been minimized.

Member

jalabort commented Jan 8, 2015

This is ready for inspection. Let me know what you guys think.

I believe things like building a large PCAModel using the method increment from a potentially very large generator of high dimensional features using a function or a class method in PCAModel should be handled in a separate PR.

@jabooth jabooth added this to the 0.5.0 milestone Jan 14, 2015

jalabort added some commits Jan 26, 2015

Merge branch 'master' of https://github.com/menpo/menpo into incremen…
…tal_pca

Conflicts:
	docs/source/api/menpo/math/index.rst
	docs/xref_map.py
	menpo/math/decomposition.py
	menpo/model/pca.py
Merge branch 'incremental_pca' of http://github.com/jalabort/menpo in…
…to incremental_pca

Conflicts:
	menpo/model/pca.py
@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Feb 10, 2015

@jabooth Are we happy with this, shall we get it in?

@jabooth

This comment has been minimized.

Member

jabooth commented Feb 10, 2015

Haven't actually reviewed it yet - I'll look it over today

Whether to use a biased estimate of the number of samples. If ``False``,
subtracts ``1`` from the number of samples.
inplace : `bool`, optional
X : (n_samples, n_dimensions) ndarray

This comment has been minimized.

@jabooth

jabooth Feb 10, 2015

Member

These docs just need to be formatted with the new doc style

Parameters
-----------
samples : list of :map:`Vectorizable`

This comment has been minimized.

@jabooth

jabooth Feb 10, 2015

Member

and docs here too

@@ -977,3 +944,37 @@ def __str__(self):
self.noise_variance(), self.noise_variance_ratio(),
self.n_components, self.components.shape)
return str_out


def extract_data(samples, n_samples=None, verbose=False):

This comment has been minimized.

@jabooth

jabooth Feb 10, 2015

Member

@jalabort this is nice how you cleaned it up but in light of @patricksnape s PR #528 we should probably use his generalised solution. Not sure which order we should bring these in?

@jabooth

This comment has been minimized.

Member

jabooth commented Feb 10, 2015

I'll bring #528 in, merge it with this, and fix the docs.

@jabooth

This comment has been minimized.

Member

jabooth commented Feb 10, 2015

@jalabort I've merged this with #528 which only changes extract_data. I have two tests failing on OS X so we'll see if it's a BLAS thing or a regression I've introduced somehow in the morning. Just the docs to fix then we can get this in.

Fix docs and axis bug
Tests were failing because the data matrix is
(n_samples, n_features) and when @jabooth refactored it he
got it backwards by accident.

Also, updated the docs for the new linalg stuff and fixed
the new linalg methods documentation.

jabooth added a commit that referenced this pull request Feb 11, 2015

@jabooth jabooth merged commit 013dc04 into menpo:master Feb 11, 2015

2 checks passed

continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci The Travis CI build passed
Details

@jabooth jabooth removed the in progress label Feb 11, 2015

@jabooth jabooth deleted the jalabort:incremental_pca branch Feb 11, 2015

@jalabort

This comment has been minimized.

Member

jalabort commented Feb 11, 2015

Hey I had already left when you comment on this yesterday. I guess this is good now since it's been merged...

Thanks @patricksnape for the docs fix ;-)

@jabooth jabooth referenced this pull request Feb 12, 2015

Merged

Init classmethods #19

@nontas nontas referenced this pull request Mar 11, 2015

Merged

Bugfix: PCAModel print #564

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment