search

Sm00thix / IKPLS

Fast CPU and GPU Python implementations of Improved Kernel PLS by Dayal and MacGregor (1997) and Shortcutting Cross-Validation by Engstrøm (2024).
https://ikpls.readthedocs.io/en/latest/
Apache License 2.0
10 stars 4 forks source link

Added support for independent centering and scaling of X and Y #20

Closed Sm00thix closed 6 months ago

Sm00thix commented 6 months ago

It is now possible to center and scale X and Y independently. It is also possible to scale without prior centering.

Sm00thix commented 6 months ago

Hi @parmentelat , Sorry to bother you again with some changes to the code base. I just want to make sure you agree with the changes before I merge them to main as you are currently reviewing this repo over at JOSS.

The motivation: After using the ikpls package myself and talking with some of my peers, I discovered the usefulness of being able to center and scale X and Y independently of eachother and also the ability to apply scaling without previous centering.

The changes: For all PLS algorithms, I replaced the center and scale parameters with the new center_X, center_Y, scale_X, and scale_Y parameters. I also modified the code so that it is possible to scale without centering. I modified the existing tests to account for this. I also added a new test that checks for equivalency between all algorithms for all 2^4=16 combinations of center_X, center_Y, scale_X, and scale_Y. I also updated the paper/time_pls.py script to account for these changes. Finally, I updated the README and all relevant documentation.

Will you allow me to merge this pull request to the main branch?

parmentelat commented 6 months ago

Hi @Sm00thix

as far as I can tell this change goes in the right direction, since it allows to do more than the previous API so please go ahead with this PR :)

-- Thierry

Sm00thix commented 6 months ago

Hi @Sm00thix

as far as I can tell this change goes in the right direction, since it allows to do more than the previous API so please go ahead with this PR :)

-- Thierry

This is correct. All of the previous functionalities are retained and new ones are added :) Thank you for the feedback. I will merge the PR.

-- Ole