layout(ring): ring_categorical_layout is non-deterministic across processes (set iteration order) and mutates the caller's order= list in place

Aperta Adatta ai principianti
#1,965 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
2/5
Tempo stimato
1-3 ore
Idoneità per principianti
68/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Tranquilla
Stack tecnologico
pandas, python

Direzione di ricerca

Leggi graphistry/layout/ring/categorical.py, in particolare le righe 55, 57 e 187-213, e riproduci il problema con lo script PYTHONHASHSEED dell’issue e l’esempio di mutazione drop_empty=False. Verifica se i test di layout esistenti coprono valori order parziali e la conservazione dell’input del chiamante. Il lavoro è completato quando gli stessi dati vengono associati ad anelli stabili tra un’esecuzione e l’altra e la lista order fornita rimane invariata.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Found by the 2026-08 layouts/viz amplification probe. Repros on master @ 42b4f7f7d, pandas 2.3.3 / numpy 2.2.6 / py3.12.

ring_categorical_layout() assigns rings for values not in the user's order by iterating a Python set. String hashing is salted per process, so the same graph and the same call produce different radii, different axis labels, and a different picture on every run — with no seed parameter and nothing in the docs saying the result is non-deterministic.

graphistry/layout/ring/categorical.py:200-208:

200:    unhandled = set(unique()) - set(order)
201:    if len(unhandled) > 0:
202:        if not combine_unhandled:
203:            if append_unhandled:
204:                order += list(unhandled)        # <-- set iteration order
205:            else:
206:                order = list(unhandled) + order

list(unhandled) over a set of strings is ordered by hash, which varies with PYTHONHASHSEED. order then feeds val_to_ring (:210-213) and val_to_r (:231) directly.

Defect 1 (non-determinism): ring assignment varies across processes

# save as ring_det.py
import pandas as pd, graphistry
e = pd.DataFrame({'s':[0,1,2],'d':[1,2,0]})
n = pd.DataFrame({'id':[0,1,2], 'cat':['alpha','beta','gamma']})
g = graphistry.edges(e,'s','d').nodes(n,'id')
out = g.ring_categorical_layout('cat', order=['alpha'], drop_empty=False)
print(','.join(f"{i}:{r}" for i, r in zip(out._nodes['id'], out._nodes['r'])))
$ for s in 0 1 2 3 4; do PYTHONHASHSEED=$s python ring_det.py; done
0:100.0,1:550.0,2:1000.0
0:100.0,1:550.0,2:1000.0
0:100.0,1:1000.0,2:550.0     <-- beta and gamma swap rings
0:100.0,1:1000.0,2:550.0
0:100.0,1:1000.0,2:550.0

Observed: beta and gamma swap rings depending on the process hash seed.
Expected: a stable order (first-appearance in the data, or sorted) — this is a deterministic categorical mapping, not a force-directed layout.

The trigger is a partial order, which is exactly the documented usage:

g2 = g.ring_categorical_layout('a_cat_node_column', order=['a', 'b', 'c'])

(categorical.py:113-120). With order=None the default order = unique() makes unhandled empty and the layout is stable; the docs' partial-order example is what breaks.

Two more places consume set order: categorical.py:55 and :57 val_to_r[next(iter(unhandled))] pick an arbitrary representative to hand the user's label callback.

Related, engine-level: categorical.py:187 g._nodes[ring_col].unique() returns first-appearance order in pandas but cuDF makes no such guarantee, so the default ring order is also expected to differ CPU vs GPU (INDICATIVE — no GPU run, see below).

Defect 2 (caller-visible mutation): order += list(unhandled) edits the caller's list in place

:204 uses += on the argument, which for a list is list.extend. The else branch at :206 correctly rebinds instead.

import pandas as pd, graphistry
e = pd.DataFrame({'s':[0,1,2],'d':[1,2,0]})
n = pd.DataFrame({'id':[0,1,2], 'cat':['a','b','c']})
g = graphistry.edges(e,'s','d').nodes(n,'id')

my_order = ['a']
g.ring_categorical_layout('cat', order=my_order, drop_empty=False)
print(my_order)

Observed: ['a', 'c', 'b'] — the caller's list has been mutated.
Expected: ['a'] unchanged.

Consequence: calling the same layout twice with the same order variable is not idempotent — the second call sees a different (already-extended) order and, because of Defect 1, a hash-dependent one. A shared module-level ORDER = [...] constant is silently corrupted for the rest of the process.

drop_empty=True (the default) hides this by rebinding order at :198; drop_empty=False exposes it. The mutation is real on both paths in the sense that += on the passed object is what runs whenever :198 did not rebind.

Category

Defect 1: undocumented non-determinism in a layout that should be a pure function of the data. Defect 2: silent-wrong caller-visible side effect on an input argument.

Not exercised

No GPU run — cupy here cannot load libnvrtc.so.12, so the ring layouts fail at import on cuDF frames for an environmental reason. The cuDF unique()-ordering claim above is static only and labelled INDICATIVE.

Lingua principale
Python
Stelle
2.6k
Fork
229
Merge medio
1g 5h
PR unite (30g)
99

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di graphistry/pygraphistry

Tutte le issue di graphistry/pygraphistry

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.