fix: tds-dpcca by MayaGavrichenkova · Pull Request #117 · Digiratory/FluctuationAnalysisTools

MayaGavrichenkova · 2026-03-12T16:58:34Z

Fix in tds-dpcca: correct comparison between analyzed points with time lag and current size of s window.

…ndow size)

StatTools/analysis/dpcca.py

kilo-code-bot · 2026-03-12T17:03:15Z

Code Review Summary

Status: 3 Issues Remaining | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	1
WARNING	1
SUGGESTION	1

Issue Details (click to expand)

CRITICAL

File	Line	Issue
`StatTools/analysis/dpcca.py`	N/A	The `tds_dpcca_worker` function was completely rewritten with new logic. The new approach uses a different algorithm (computing covariation across all lags simultaneously then extracting diagonals) instead of the original per-lag computation. While the author has provided detailed explanations in PR comments, this remains a significant behavioral change that warrants thorough testing.

WARNING

File	Line	Issue
`StatTools/analysis/dpcca.py`	198	New code raises `ValueError("Use lags")` if neither `time_delays` nor `max_time_delay` is provided. The old code returned `None` implicitly, which had different caller implications. Verify this is the intended behavior change.

SUGGESTION

File	Line	Issue
`tests_manual/tds_dpcca.py`	N/A	Manual test file uses `print()` statements for output. Consider using proper logging or returning values for programmatic inspection.

Resolved Issues

The following previously identified issues have been addressed:

✅ Typo "indieces" → "indices" (fixed in new code)
✅ Debug print statements in _partial_correlation (removed)
✅ Using _covariation_single_signal as recommended
✅ Added rcond=None to np.polyfit call (fixes deprecation warning)
✅ Added justification comment for np.linalg.pinv usage
✅ Unused variable issue in test file (addressed)

Other Observations (not in diff)

Issues found in unchanged code that cannot receive inline comments:

File	Line	Issue
`StatTools/analysis/dpcca.py`	52	Debug print statement in `_partial_correlation`: `print(f" Error: Sqrt(-1)! No P array values for this S!")` - This error handling uses print instead of logging. Consider using proper logging or removing for production.

Files Reviewed (3 files)

StatTools/analysis/dpcca.py - 3 issues (1 critical, 1 warning, 1 suggestion)
tests/test_dpcca.py - Test parameters updated for new behavior
tests_manual/tds_dpcca.py - New manual test file added

Note: The author (MayaGavrichenkova) has provided detailed explanations of the tds-dpcca algorithm changes in PR comments. The remaining critical issue is about the architectural change in how the algorithm processes time lags, which the author has defended as correct. Reviewers should carefully evaluate the algorithm explanation against the intended mathematical behavior.

MayaGavrichenkova · 2026-03-13T15:59:01Z

Обоснование текущей логики и объяснение, почему ввод сравнения intersection_length!=s_val некорректен. Требуется анализировать количество точек глобально(внутри всего массива) и локально(внутри текущего временного окна).
Есть массив положений стартов временных окон в исходном массиве внутри цикла по временным масштабам s_val : start = np.arange(0, n - s_val + 1). Далее скользящим окном идем по всем временным окнам с определенным шагом signal_view = signal_view[:, :: int(step * s_val), :]. В цикле по всем временным лагам создается цикл по всем индексам временных окон.
Находится позиция начала текущего временного окна, как start_pos = start_window[w].

При лаге>0: находится значение положение глобального конца, как минимум между (позиция начала временного окна+колличество точек во временном масштабе, длина общего входного массива-значение лага). Также значение позиции локального конца находится, как (колличество точек во временном масштабе-значение лага). intersection_length это длина итогового массива точек для анализа, которая находится как минимум(значение позиции глобального конца-значение позиции начала текущего временного окна, значение количества точек, которое находится как колличество точек во временном масштабе-значение лага). shift_sig=0 это значение позиции сигнала без лага, shift_sig_lag=lag значение позиции сигнала с лагом.

При лаге <0: находится значение положение глобального старта, как max(значение положения начала текущего окна, значение положения начала текущего окна-значение лага). Значение положения глобального конца находится, как (значение положения начала текущего окна+ значение точек во временном масштабе). intersection_length находится как (значение положения глобального конца-значение положения глобального старта). shift_sig = -lag - значение позиции сигнала без лага, shift_sig_lag = 0 - значение сигнала с лагом.

Возникала ошибка с недостатком точек для polyfit: реализована проверка на количество точек: окна, где точек недостаточно-откидываются.

Для каждого сигнала находим итоговое колличество точек с лагом и без лага в конкретном окне: data_true = signal_view[sig_idx, w, shift_sig : shift_sig + intersection_lenght] -данные без лага для w-окна, кол-во точек для анализа. data_lag_true = signal_view[sig_idx, w, shift_sig_lag : intersection_lenght + shift_sig_lag] - данные c лагом для w-окна, кол-во точек для анализа.

Почему ввод сравнения intersection_length!=s_val некорректен: так как intersection_length - это количество точек для анализа в текущем временном окне для исходного сигнала и сигнала с временным лагом, приведем пример. Исходный сигнал с индексами значений во временном окне (0...10) при лаге=5, следовательно, индексы для сдвинутого сигнала будут (5..15). Значение пересечения (5..10) =6 позиций при изначальном значении длины временного окна=11. Следовательно, такое сравнение будет выполняться при все лагах кроме 0, что приводит к некорректной работе tds-dpcca.

StatTools/analysis/dpcca.py

Sinitca-Aleksandr · 2026-03-13T17:19:43Z

Почему ввод сравнения intersection_length!=s_val некорректен: так как intersection_length - это количество точек для анализа в текущем временном окне для исходного сигнала и сигнала с временным лагом, приведем пример. Исходный сигнал с индексами значений во временном окне (0...10) при лаге=5, следовательно, индексы для сдвинутого сигнала будут (5..15). Значение пересечения (5..10) =6 позиций при изначальном значении длины временного окна=11. Следовательно, такое сравнение будет выполняться при все лагах кроме 0, что приводит к некорректной работе tds-dpcca.

s_val -- это число отсчетов в анализируемом масштабе. Анализ других по длине окон -- это другие масштабы. В начале и конце сигнала варианты запаздываний, очевидно, ограничены -- это нормально.

StatTools/analysis/dpcca.py

tests/test_dpcca.py

Sinitca-Aleksandr · 2026-03-19T17:15:25Z

tests_manual/tds_dpcca.py

+    p, r, f, s = dpcca(
+        signal.T,
+        pd=1,
+        step=1,
+        s=s_val,
+        time_delays=[-60, -50, -40, 0, 40, 50, 60],
+        n_integral=1,
+    )


зачем каждый раз считать dpcca?

Нужен ли этот тест в репозитории вообще? Автотесты делают примерно то же самое. А пример будет голубиный чуть позже

Этот тест уберу, он правда повторяет предыдущий

Не убрала

pipipyau · 2026-03-20T08:57:23Z

StatTools/analysis/dpcca.py

+                lag_length = end - start
+                cur_window = (lag_length - s_val) // step_s + 1
+                n_windows_arr.append(cur_window)
+        n_windows = min(n_windows_arr)


Может вызвать ValueError: min() iterable argument is empty если в список ничего не добавится

Добавила

Чёт ты странное сделала. Что ты пыталась сделать то?
Ася говорит, что выполнение команды на 232 строке может вызвать исключение при определенных условиях

Да, можно было добавить if n_windows_arr: n_windows = min(n_windows_arr)

StatTools/analysis/dpcca.py

pipipyau · 2026-03-20T08:59:31Z

tests/test_dpcca.py

-    for r_pred in r:
-        np.testing.assert_allclose(r0, r_pred, atol=0.2)
+        estimated_lag = time_delays[max_lag_idx]
+        assert abs(estimated_lag - (-true_lag)) <= 10


<= 10 не слишком много?

Я там увеличила порядок из-за увеличения порядка временных лагов. Был лаг 50->теперь 50

Может, тогда сделать не абсолютное значение значение?

Был лаг 50->теперь 50

?

StatTools/analysis/dpcca.py

tests_manual/tds_dpcca.py

pipipyau · 2026-03-20T09:05:54Z

tests/test_dpcca.py

+    pd = 2
+    n_integral = 1
+    true_lag = 50
+    p, r, f = tds_dpcca_worker(


неиспользуемые переменные

Внесла в tdc_dpcca_worker неиспользуемые

p и f неиспользуемые

https://cwe.mitre.org/data/definitions/563.html

Вернуть _, r, _ = tds_dpcca_worker(, чтобы p, f не присвоились

pipipyau · 2026-03-20T09:19:52Z

tests/test_dpcca.py

    for s_idx in range(len(s)):
-        correlation = r[:, s_idx, 0, 1]
+        correlation = r[:, s_idx, :, :]
        max_lag_idx = np.argmax(correlation)


А почему тут сменились индексы?

Тест исправила

Это ответ о причине изменения индексов или о том, что ты поправил по замечанию Аси?

kilo-code-bot

Code Review Summary

Status: 8 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	4
WARNING	2
SUGGESTION	2

Issue Details (click to expand)

CRITICAL

File	Line	Issue
`StatTools/analysis/dpcca.py`	231	The code raises `ValueError("min() iterable argument is empty")` for ANY invalid time lag, instead of gracefully skipping that lag. This is a regression from the old code which would check each window individually and skip invalid ones. This will cause the program to crash for edge cases where some lags are naturally invalid at the data boundaries.
`StatTools/analysis/dpcca.py`	266	The `_covariation` function that handled two signals was removed. The PR now uses `_covariation_single_signal` for all cases, which may not correctly handle correlations between two different signals.
`StatTools/analysis/dpcca.py`	N/A	The `tds_dpcca_worker` function was completely rewritten with new logic. The new approach uses a different algorithm (computing covariation across all lags simultaneously then extracting diagonals) instead of the original per-lag computation. This is a significant behavioral change.
`StatTools/analysis/dpcca.py`	368	Significant behavior change - removed the `(processes == 1 or len(s) == 1)` condition. This means `tds_dpcca_worker` will now be called even when `processes > 1` and `len(s) > 1`, which may have performance implications or change intended behavior.

WARNING

File	Line	Issue
`StatTools/analysis/dpcca.py`	120	New validation check `if start_arr + end_arr >= n` raises ValueError earlier than the old code. This changes when errors are raised for invalid time delays.
`StatTools/analysis/dpcca.py`	131	Changed from checking `s_val > n` to `s_val > valid_arr_lag`. This is a logic change that may affect edge cases.

SUGGESTION

File	Line	Issue
`StatTools/analysis/dpcca.py`	237	Typo in comment - "indecies" should be "indices"
`StatTools/analysis/dpcca.py`	46	The `_partial_correlation` function uses `np.linalg.inv(R)` which will raise `np.linalg.LinAlgError` if R is singular - consider adding error handling for edge cases.

Files Reviewed (3 files)

StatTools/analysis/dpcca.py - 8 issues
tests/test_dpcca.py - Test parameters updated for new behavior
tests_manual/tds_dpcca.py - New manual test file added

Fix these issues in Kilo Cloud

Sinitca-Aleksandr · 2026-03-20T17:58:06Z

StatTools/analysis/dpcca.py

+                lag_length = end - start
+                cur_window = (lag_length - s_val) // step_s + 1
+                n_windows_arr.append(cur_window)
+        n_windows = min(n_windows_arr)


Чёт ты странное сделала. Что ты пыталась сделать то?
Ася говорит, что выполнение команды на 232 строке может вызвать исключение при определенных условиях

Sinitca-Aleksandr · 2026-03-20T17:58:53Z

tests_manual/tds_dpcca.py

+    p, r, f, s = dpcca(
+        signal.T,
+        pd=1,
+        step=1,
+        s=s_val,
+        time_delays=[-60, -50, -40, 0, 40, 50, 60],
+        n_integral=1,
+    )


Не убрала

pipipyau · 2026-03-23T07:33:55Z

StatTools/analysis/dpcca.py

+                lag_length = end - start
+                cur_window = (lag_length - s_val) // step_s + 1
+                n_windows_arr.append(cur_window)
+        n_windows = min(n_windows_arr)


Да, можно было добавить if n_windows_arr: n_windows = min(n_windows_arr)

pipipyau · 2026-03-23T07:34:41Z

tests/test_dpcca.py

+    pd = 2
+    n_integral = 1
+    true_lag = 50
+    p, r, f = tds_dpcca_worker(


Вернуть _, r, _ = tds_dpcca_worker(, чтобы p, f не присвоились

pipipyau · 2026-03-23T07:38:24Z

tests_manual/tds_dpcca.py

+# Generate fractional Brownian noise using the default Kasdin method
+sig_1 = generate_fbn(hurst=hurst, length=length)
+sig_2 = generate_fbn(hurst=hurst, length=length)
+print(sig_2.shape)


Отладочные принты

pipipyau · 2026-03-23T07:40:46Z

tests/test_dpcca.py

-    for r_pred in r:
-        np.testing.assert_allclose(r0, r_pred, atol=0.2)
+        estimated_lag = time_delays[max_lag_idx]
+        assert abs(estimated_lag - (-true_lag)) <= 10


Может, тогда сделать не абсолютное значение значение?

Был лаг 50->теперь 50

?

MayaGavrichenkova · 2026-03-23T11:01:23Z

Объяснение концепции tds-dpcca: start_arr = max(0, -min_lag) end_arr = max(0, max_lag) - кол-во точек, которые надо обрезать в массиве при положительном и отрицательном лаге(в начале или в конце). valid_arr_lag = n - start_arr - end_arr - кол-во точек массива, которые остаются для анализа после смещения на временной лаг. Далее для каждого лага вычисляем сколько временных окон поместится:
start = start_arr + lag end = start + valid_arr_lag , где Start-это начало среза с текущим лагом и end- это конец среза с текущим лагом. Тк valid_arr_lag одинаков для всех лагов, то так можно сделать, это гарантирует одинаковую длину окон для sliding window для избежания ошибок размерности. lag_length - длина среза с текущим лагом. cur_window = (lag_length - s_val) // step_s + 1 - кол-во скользящих окон в текущем срезе (кол-во точек всего с лагом- размер окна)/шаг+1. Добавляем +1 чтобы учитывать первое окно, так как оно начинается с позиции 0. n_windows = min(n_windows_arr) if n_windows_arr else 0 - берем значение с минимальным кол-вом окон, так как для разных лагов может быть разное кол-во окон и возможна ошибка размерности.
В цикле по всем лагам опять вычисляем срез анализируемых данных. shifted_arr= cumsum_arr[:, start:end] - извлекаем данные для обоих сигналов в срезе для текущего лага. windows = np.lib.stride_tricks.sliding_window_view(shifted_arr, window_shape=s_val, axis=1) - создаем скользящее окно для уже массива с лагами с текущим размером окна. windows-прореживаем окна с определенным шагом.
if windows.shape[1] >= n_windows:
all_windows[lag_index, :, :, :] = windows[:, :n_windows, :]
else:
useful_window_shape = windows.shape[1]
all_windows[lag_index, :, :useful_window_shape, :] = windows[
:, :useful_window_shape, :
]

проверка, если текущее кол-во окон больше или равно оптимальному минимальному, вычисленному раннее, то сохраняем первые n_windows окон в all windows, это гарантирует одинаковый размер окон для всех лагов. Если текущее ко-во окон меньше, то сохраняем это кол-во в all windows.
Детренд:
detrended = np.zeros((n_lags * n_signals, n_windows * s_val))
массив по всем сигналам и всем точкам во всех окнах. В цикле по всем сигналам со всеми лагами во всех окнах производим детренд. detrended data - детрендированные данные в текущем окне с размером с s_val. Чтобы произвести дернет в следующем окне совершаем position_idx += s_val.
for D in range(n_lags):
f[D, s_i, :, :] = covariation_lag[D, :, D, :]
r[D, s_i, :, :] = correlation_lag[D, :, D, :]
p[D, s_i, :, :] = partial_correlation_lag[D, :, D, :]
для каждого лага выделяем ковариацию и тд. Берём диагональные блоки, так как они совпадают для одних и тех же лагов.

fix: tds-dpcca (comparison betweeb analyzed points with lags and s wi…

9d3d20d

…ndow size)

MayaGavrichenkova requested a review from Sinitca-Aleksandr March 12, 2026 16:58

MayaGavrichenkova self-assigned this Mar 12, 2026

kilo-code-bot bot reviewed Mar 12, 2026

View reviewed changes

Sinitca-Aleksandr added the bug Something isn't working label Mar 12, 2026

Sinitca-Aleksandr added this to the Release 1.11.0 milestone Mar 12, 2026

fix: size of window with analyzed points

889f7f5

correct comparison for analyzed data

0d0e331

kilo-code-bot bot reviewed Mar 13, 2026

View reviewed changes

StatTools/analysis/dpcca.py Outdated Show resolved Hide resolved

MayaGavrichenkova added 2 commits March 16, 2026 00:49

tds-dpcca by s_val shift

cbd610a

fix: tds-dpcca

f750130

Sinitca-Aleksandr reviewed Mar 19, 2026

View reviewed changes

StatTools/analysis/dpcca.py Outdated Show resolved Hide resolved

fix tds-dpcca: one signal to covariation

f654531

kilo-code-bot bot reviewed Mar 19, 2026

View reviewed changes

StatTools/analysis/dpcca.py Outdated Show resolved Hide resolved

StatTools/analysis/dpcca.py Outdated Show resolved Hide resolved

Merge branch 'main' into fix-tds-dpcca-window_size

2f8702f

Sinitca-Aleksandr requested changes Mar 19, 2026

View reviewed changes

Sinitca-Aleksandr requested a review from pipipyau March 19, 2026 17:16

fix tds-dpcca: removed debug stuff

ad5c756

pipipyau reviewed Mar 20, 2026

View reviewed changes

MayaGavrichenkova added 2 commits March 20, 2026 16:54

fix tds-dpcca: test bugs

652029a

fix test tds dpcca

ad58084

kilo-code-bot bot reviewed Mar 20, 2026

View reviewed changes

fix test tds

4e2ed01

Sinitca-Aleksandr requested changes Mar 20, 2026

View reviewed changes

pipipyau self-requested a review March 23, 2026 05:42

pipipyau reviewed Mar 23, 2026

View reviewed changes

MayaGavrichenkova and others added 2 commits March 24, 2026 16:32

fix tds-dpcca: analyse base signal and shifted signal

f5185b7

Merge branch 'main' into fix-tds-dpcca-window_size

0131515

Conversation

MayaGavrichenkova commented Mar 12, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kilo-code-bot bot commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Summary

Overview

CRITICAL

WARNING

SUGGESTION

Uh oh!

MayaGavrichenkova commented Mar 13, 2026

Uh oh!

Uh oh!

Sinitca-Aleksandr commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MayaGavrichenkova Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kilo-code-bot bot left a comment

Choose a reason for hiding this comment

Code Review Summary

Overview

CRITICAL

WARNING

SUGGESTION

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

kilo-code-bot bot commented Mar 12, 2026 •

edited

Loading

Sinitca-Aleksandr commented Mar 13, 2026 •

edited

Loading

MayaGavrichenkova Mar 20, 2026 •

edited

Loading

MayaGavrichenkova commented Mar 23, 2026 •

edited

Loading