gridfm / gridfm/gridfm-graphkit
Exported bus / idx columns are positional, not real IDs
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 105
- Forks
- 36
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 9
Description
local_index_per_graph (tasks/utils.py, added in #87) regenerates per-graph indices from bincount(batch). Both predict_steps export these as the bus and idx identity columns in every parquet — predictions and now embeddings.
These are positions, not IDs. Correctness depends on datakit emitting buses in order and the wrapper's groupby preserving it. If that ever changes, the exported IDs silently mislabel rows.
Buses are at least validated (powergrid_hetero_dataset.py:162 asserts against the real bus column). Generators are not — gen_index is set from the positional DataFrame index (reset_index()), is never stored on the graph, and has no assert. Both spots carry TODOs saying to store real IDs in the tensors.
Fix: carry the true bus/gen IDs as tensor fields from the dataset onward and export those instead of regenerated positions; the ordering assert can then go away.
Refs #87
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with local_index_per_graph in tasks/utils.py and trace how graph fields flow from the dataset into both predict_step exports. Inspect powergrid_hetero_dataset.py:162 and the generator TODOs, then verify that parquet bus and idx columns use stored true IDs rather than regenerated positions and that the ordering assert is no longer needed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- data
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100