Grouped-sum absence rules are implemented twice (CSRStore._from_scatter and CSRStore.aggregate)

Abierto
  • ¿Algo no está bien?
#1,124 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
25/100
Tipo de issue
Refactorización
Claridad
Bastante claro
Estado de actividad
Activo
Stack tecnológico
python
Área
backend

Línea de trabajo

The two grouped-sum kernels are CSRStore._from_scatter and CSRStore.aggregate in linopy/csr.py, reached through from_grouper, and CSRStore.from_coo is a third presence rule. Start by reading convention.rst §13 and comparing how each kernel derives presence and the constant. The lower-effort path is a conformance test that runs groupby().sum() through both kernels over the listed absence cases and asserts identical stores. Refactoring _from_scatter onto aggregate is design work and should be agreed with maintainers first.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

[!NOTE]
The following content was generated by AI.

CSRStore has two kernels that compute a grouped sum, and each one implements the absence rules (convention.rst §13) on its own:

  • CSRStore._from_scatter (linopy/csr.py, used by from_grouper for dense sources) scatters the dataset's terms into cells. It drops -1 / NaN terms, then derives presence and the constant itself: const[cell_rows] = 0.0 + np.add.at(...), with a skipna switch.
  • CSRStore.aggregate (linopy/csr.py, used by from_grouper for CSR sources and by DenseStore.aggregate via as_sparse(...).aggregate(...).to_dense()) regroups CSR rows. It derives presence from the row bounds (const[np.diff(bounds) == 0] = np.nan) and always skips NaN constants.

Today both agree on every case we probed: cells nothing lands in are absent, cells whose members are all absent are present with constant 0, and NaN constants are skipped. But the rules live in two places. A change to one won't reach the other, and dense and sparse groupby().sum() could start to diverge silently. This came up in the review of #1122, where a third copy of the presence rule (CSRStore.from_coo) had already drifted: cells reached only through masked labels came out absent instead of zero.

Possible directions

  • Express _from_scatter as "build one CSR row per member, then aggregate", so presence and constant handling live in aggregate alone. The skipna=False case would remain a small post-step on the constant.
  • Or, at minimum, add a conformance test that runs groupby().sum() through both kernels over a table of absence cases (unaddressed cell, all-masked members, NaN member constants, explicit zero coefficients, duplicates) and asserts identical stores.

Related observations, possibly separate

  • §13 says "a sum of none is the zero expression", but a multi-key cartesian groupby and reindex give absent cells, while @ / .sum() give zero. The convention docs should say which rule applies where.
  • At the constraint layer, a present row with no live terms is silently dropped (active_row_mask, constraints.py). An all-masked 0 == 1 therefore disappears instead of making the model infeasible, and the solver can't tell absent from zero.

Generated by Claude Code

Lenguaje dominante
Python
Estrellas
263
Forks
93
Merge medio
1 d 10 h
PR fusionados (30 d)
68

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de PyPSA/linopy

Todos los issues de PyPSA/linopy

Issues similares

Más issues de Python

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.