Repository navigation
perf(model): relabel assign_coords containers without a label dict - #1030
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
14e9d58 to
824d1f0
Compare
dbf4c67 to
663a15c
Compare
|
Rebased onto
The gain is about 100x, versus 280x in the original benchmark. |
663a15c to
de4ade1
Compare
|
I fail to understand the changes when scanning over them. is the problem perhaps within |
Model.assign_coords rebuilt a master->new dict per container and mapped each container's labels through Index.map. On a tz-aware DatetimeIndex that map falls into pandas' object-to-datetime inference, costing milliseconds per container. Map full-length containers straight to the new values and locate subset containers' labels with get_indexer/take, and build a reassigned Dataset once instead of twice. ~817 ms -> ~3 ms for 30 containers on a 4380-label tz-aware axis. Co-Authored-By: Claude Code <noreply@anthropic.com>
Drop the inline comments, the `equals` fast path and the redundant `.rename(dim)`: `get_indexer` returns arange for a full-length index, and `take` preserves the name. Update the test docstring and the benchmark numbers in the release note. Co-Authored-By: Claude Code <noreply@anthropic.com>
d1fba72 to
314a6ed
Compare
|
@FabianHofmann , yes, the slowness is pandas' datetimelike inference. Inline comments removed & code simplified. The 'equals' branch is gone as benefits are super tiny, not worth an extra branch. Best of 5: 809 ms on master, 11.5 ms with the relabel alone, 8.8 ms with both changes. The PR description now has the before/after and this table. |
Taking assign_coords for a spin, I realised it was way too slow. this PR fixes it. Sorry for not catching this sooner...
Note
The following content was generated by AI.
What
Model.assign_coords(from #955) relabelled containers by mapping each label throughIndex.map(dict(zip(master, new))), and rebuilt the dict once per container.Two changes, both inside the #955 code:
Positional relabel.
master.get_indexer(index)locates each container's labels andnew.take(positions)selects the new values.get_indexeralso replaces the separateisin(master).all()check, so one lookup does both jobs.On a tz-aware
DatetimeIndex, the rolling-horizon case the API is built for,Index.mapreturns an object array of Timestamps and pandas infers datetime64 back. That inference accounts for 16 ms of the 20 ms per container on pandas 3.1.0 (objects_to_datetime64), and the dict rebuild adds 6 ms.One
Datasetconstruction.reassign_coordswrites the new index into the coordinate's original position and builds the dataset once, instead ofDataset.assign_coords(...)followed by a secondDataset(...)to restore the coordinate order. This is a small second gain (about 3 ms), not part of the speedup above.Benchmark
30 containers (15 variables + 15 CSR constraints), 4380 tz-aware labels,
Model(sparse=True), pandas 3.1.0, a fresh model per call, interleaved, best of 5:Model.assign_coordsmaster)Datasetconstruction onlyReproduction
Checklist
AGENTS.md).doc.doc/release_notes.rstof the upcoming release is included.