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

Add a facewise (tensor) version of the crossprod() function #317

Merged
merged 6 commits into from
Oct 6, 2024
Merged

Add a facewise (tensor) version of the crossprod() function #317

merged 6 commits into from
Oct 6, 2024

Conversation

brendanlu
Copy link
Contributor

@brendanlu brendanlu commented Oct 3, 2024

In PLS-like analysis, operations of the type $X^T Y$ are performed pretty often, so it clocked me earlier today that it would probably be worth adding a simple tensor version of this to save some computation time.

UPDATE: for some reason it's actually slower on benchmarks so putting this on hold...R's crossprod() is slower on my machine even for matrix inputs.

Minor: also added a parameter check and unit test for tplsda

FINAL NOTE

See https://stackoverflow.com/questions/79049231/crossprodm1-m2-is-running-slower-than-tm1-m2-on-my-machine. I asked this question a few days back and there's some pretty handy benchmarks and discussion from some knowledgeable people, but no definitive reason / answer as of yet.

This is actually a more complex issue than I can bother for now, as it seems the default NETLIB linalg routines that ships by default with R (on both Mac and Windows as we have tested) actually result in crossprod() being slower, which I found when benchmarking the new facewise crossproduct version which is resultingly slower too. I'm not sure how complex or odd this issue may be, but it seems that this might be fixed in the future potentially even upstream in R, who knows? If this happens I'll change tpls at a later date to use the (meant to be quicker) crossprod() based version.

My guess is most MixOmics2 users are probably going to be relying on these NETLIB linalg routines on a personal computer without linking to any fancier BLAS or LAPACK libs, there'll be no changes to tpls I'll just commit this new function and its unit tests as a public function that's probably a bit useless for now. Also, a good lesson learnt for me in making premature (probably unneeded) optimizations.

@brendanlu brendanlu marked this pull request as ready for review October 5, 2024 04:47
@evaham1 evaham1 merged commit 2840a15 into mixOmicsTeam:master Oct 6, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants