cosanlab / cosanlab/nltools

make_cosine_basis(sampling_freq=) takes a sampling interval in seconds, not a frequency

Closed
#550 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Python
Stars
130
Forks
48
Avg merge
6h 42m
Merged PRs (30d)
34

Description

`nltools.algorithms.make_cosine_basis(nsamples, sampling_freq, filter_length, ...)` reads `sampling_freq` as the sampling interval in seconds (the TR), not a frequency in Hz: the SPM-style formula it implements is `fix(2 * n * RT / HParam + 1)`, and `DesignMatrix.add_dct_basis` passes `1 / sampling_freq` to it, which is why the design-matrix path is correct. A user calling the standalone function with a real frequency (0.5 Hz for a 2 s TR) gets the wrong number of regressors.

The #535 sweep (finding G-08) kept the arithmetic as v0.5.1 had it, because flipping the unit of a user-facing keyword is a breaking change, and corrected the docstring to say the argument is a sampling interval in seconds. That is a poor resting state: the keyword name says the opposite of what it takes.

Options for 0.6.1: rename the parameter to `sampling_interval` with a deprecation shim; or make the standalone function take a real frequency and have `add_dct_basis` pass `sampling_freq` straight through, with a migration row. Either needs a decision and a migration row; the S-G verification report holds the reproduction.

Contributor guide

Open the contributing guide

Research direction

Start with the S-G verification report's reproduction, then inspect make_cosine_basis and DesignMatrix.add_dct_basis to compare their sampling units. Resolve which API direction to adopt, verify the standalone and design-matrix behavior, and add the required migration row.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend-api-design
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.