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

Issues with GitHub Actions macos-latest (macos-14) runner #33

Closed Sm00thix closed 4 months ago

Sm00thix commented 4 months ago

Hi @parmentelat,

I noticed some strange issue and I would like to get your input as to how to address it.

In relation to my https://github.com/Sm00thix/IKPLS/issues/24#issuecomment-2195824075 I decided to also upgrade the macos runners from macos-12 to macos-latest for testing GitHub actions.

Annoyingly, testing with macos-latest (macos-14) using GitHub actions causes the test test_sanity_check_pls_regression_constant_column_Y to fail as evident by this run. The errors are not present on the previous version of the macos runner - i.e., macos-12 (macos-13 runners do not exist unless you are enterprise) as evident by this run. The errors are also not present on any of ubuntu-latest and windows-latest. I have a Mac of my own, running macos 14, where I am unable to reproduce the error, and the test passes as it should.

I only have two plausible explanations for this and I am unable to verify either:

  1. The macos-14 runners are running on arm64 processors whereas the macos-12 runners are running on x64 processors. My own Mac is also running on an x64 processor. Is this some freaky edge case where the different processors cause different behavior?
  2. The macos-14 runners may simply be faulty. I have had issues with them previously when they were newer (they are still only a few months old).

Should I write something about this somewhere or just assume that it's an error with the runner? What is best practice here?

Sm00thix commented 4 months ago

The issue seems to be related to the comparison between 0 and the standard deviation of X and/or Y. If 0, the standard deviation should be changed to 1. This was done using a check like std==0 using exact comparison. In general, this is not recommended for floats due to finite precision. Indeed, the finite precision was the cause of the issue mentioned in https://github.com/Sm00thix/IKPLS/issues/33#issue-2379390588. Instead we are now checking if abs(std) <= epsilon comparing std to the machine epsilon for the input dtype. https://github.com/Sm00thix/IKPLS/commit/f1a60ea5de7e736c88d2a431e9f813973fd750c6 contains the updated code with the fixes and changes the GitHub Actions tests to run on macos-latest again instead of macos-12.

Thanks to @calex22 for helping resolve this!

Sm00thix commented 4 months ago

The fixes are merged to main as of https://github.com/Sm00thix/IKPLS/commit/0ddea86570f2fb1aa5c498e0fe2d4e1cee05538b. Closing this issue.