feat: add sparse PCA #170

nicrie · 2024-07-16T15:04:16Z

Add Sparse PCA using Variable Projection.

Paper: http://arxiv.org/abs/1804.00341
Repository: https://github.com/erichson/ristretto/tree/master/ristretto

PR provides

implementation of Sparse PCA
tests for basic functionalities
basic example in documentation

Loading submodules via the xeofs package introduces circular imports. Use relative import instead to avoid these cases.

In addition to randomized approach, this alogithm provides an exact solution but suffers computational power. Only useful for small datasets

nicrie · 2024-07-16T15:34:58Z

I'm a bit perplexed as to why ruff is complaining here. The lines it complains about seem to have been tackled by your recent PR #169, specifically by commit 6b275f5.

However inspecting the current code

xeofs/xeofs/models/rotator_factory.py

Lines 47 to 53 in a2669b8

    
           if type(model) == EOF: 
        
               return EOFRotator(**self.params) 
        
           elif type(model) == ComplexEOF: 
        
               return ComplexEOFRotator(**self.params) 
        
           elif type(model) == MCA: 
        
               return MCARotator(**self.params) 
        
           elif type(model) == ComplexMCA:

it seems that these changes weren't adopted ? Do you know what happened here @slevang ?

EDIT: Nevermind, my bad, you actually updated tests/models/test_eof_rotator.py but ruff complains about xeofs/models/rotator_factory.py. So I'll fix this file here quickly.

isinstance() caters for inheritance, thus it cannot distinguish between EOF and ComplexEOF model. Instead use type() for comparison.

slevang · 2024-07-16T16:37:48Z

Not sure exactly why ruff didn't complain before on rotator_factory.py but it looks like E721 was added and stabilized recently. So I assume it is just the difference between latest ruff versions, since we didn't pin the install in the CI check.

slevang

This looks very cool, nice work! I don't have time to dig in right now but will definitely revisit this. At a glance the implementation seems solid.

nicrie added 9 commits July 16, 2024 14:26

feat: add sparse PCA

eca1efd

chore: fix package import

8594bb8

Loading submodules via the xeofs package introduces circular imports. Use relative import instead to avoid these cases.

docs: add acknowledgements to README

ce4d523

docs: fix math equations in docstrings

2fe8ace

docs: update documentation

6bbfc08

feat: add exact Sparse PCA solution

c9edb69

In addition to randomized approach, this alogithm provides an exact solution but suffers computational power. Only useful for small datasets

test: add test cases for sparse PCA

d9d6610

docs: add example for sparse PCA

18ee958

docs: add example to documentation

8434985

nicrie changed the title ~~Sparse_pca~~ feature: add sparse PCA Jul 16, 2024

nicrie changed the title ~~feature: add sparse PCA~~ feat: add sparse PCA Jul 16, 2024

nicrie added 2 commits July 16, 2024 18:00

fix: ruff compatible comparisons

e39923f

fix: use correct type comparison

4e5a87f

isinstance() caters for inheritance, thus it cannot distinguish between EOF and ComplexEOF model. Instead use type() for comparison.

nicrie marked this pull request as ready for review July 16, 2024 16:39

nicrie requested a review from slevang July 16, 2024 16:39

slevang approved these changes Jul 17, 2024

View reviewed changes

nicrie merged commit e2ccc76 into main Jul 18, 2024
6 checks passed

nicrie deleted the sparse_pca branch August 25, 2024 10:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add sparse PCA #170

feat: add sparse PCA #170

nicrie commented Jul 16, 2024

nicrie commented Jul 16, 2024 •

edited

Loading

slevang commented Jul 16, 2024

slevang left a comment

feat: add sparse PCA #170

feat: add sparse PCA #170

Conversation

nicrie commented Jul 16, 2024

nicrie commented Jul 16, 2024 • edited Loading

slevang commented Jul 16, 2024

slevang left a comment

Choose a reason for hiding this comment

nicrie commented Jul 16, 2024 •

edited

Loading