mathurinm / celer

Fast solver for L1-type problems: Lasso, sparse Logisitic regression, Group Lasso, weighted Lasso, Multitask Lasso, etc.
https://mathurinm.github.io/celer/
BSD 3-Clause "New" or "Revised" License
198 stars 32 forks source link

`climate._target_region` incorrectly extracts misaligned column #278

Open shimon-sato opened 1 year ago

shimon-sato commented 1 year ago

There seems to be a problem with the climate.target_region function which incorrectly extracts one column to the right of the intended one. https://github.com/mathurinm/celer/blob/main/celer/datasets/climate.py#L60-L72

The pos_Lx value range is supposed to be 0~143, but it is 0~144. For example, if Lx is 359, pos_Lx will be 144.

Shouldn't we use np.floor insted of np.ceil?

Badr-MOUFAD commented 1 year ago

Hi @shimon-sato, thanks for reporting that. Could you provide a code snippet that reproduces this behavior?

shimon-sato commented 1 year ago

Thanks for the reply. Reproduce codes are listed below.

Testing Environment

minimum example

import numpy as np

Lx = 359
pos_Lx = (np.ceil(Lx / 2.5)).astype(int)
assert 0 <= pos_Lx <= 143

result

AssertionError

via climate._target_region

from numpy import testing
from celer import datasets

_, y1 = datasets.climate._target_region(90, 359)
_, y2 = datasets.climate._target_region(87.5, 0)
# This code means `assert not assert_array_equal`
testing.assert_raises(AssertionError, testing.assert_array_equal, y1, y2)  

result

AssertionError: AssertionError not raised by assert_array_equal