Skip to content

perf: direct CSR-to-LP writer for frozen constraints#631

Open
FBumann wants to merge 1 commit intoPyPSA:perf/matrix-accessor-rewritefrom
FBumann:perf/matrix-accessor-rewrite+lp-writer
Open

perf: direct CSR-to-LP writer for frozen constraints#631
FBumann wants to merge 1 commit intoPyPSA:perf/matrix-accessor-rewritefrom
FBumann:perf/matrix-accessor-rewrite+lp-writer

Conversation

@FBumann
Copy link
Collaborator

@FBumann FBumann commented Mar 23, 2026

Summary

  • Override Constraint.to_polars() to expand CSR data directly into a polars DataFrame, bypassing the expensive mutable() → xarray Dataset reconstruction
  • Override Constraint.iterate_slices() to yield CSR row-batches instead of relying on xarray's isel() (which doesn't exist on frozen constraints — this was crashing)
  • Move eliminate_zeros() to freeze time (from_mutable) so cleanup happens once

Benchmark results (vs master)

Benchmark master PR #630 (before) + this PR vs master
basic n=100 12.5ms 14.1ms (+12%) 8.5ms -32%
expr_arith n=100 16.5ms 14.7ms (-11%) 10.7ms -35%
knapsack n=10k 4.2ms 4.8ms (+16%) 3.3ms -21%
sparse_net n=100 5.4ms 7.9ms (+47%) 4.6ms -15%
pypsa_scigrid 10 crashed 32.0ms fixed

Test plan

  • pytest test/test_io.py — all 25 tests pass
  • pytest test/test_constraint.py — all 72 tests pass
  • New test test_to_file_lp_frozen_vs_mutable verifies byte-identical LP output between frozen and mutable paths
  • benchmarks/test_lp_write.py — all benchmarks pass including previously crashing pypsa_scigrid

🤖 Generated with Claude Code

Override Constraint.to_polars() to expand CSR data directly into a
polars DataFrame, bypassing the expensive mutable() → xarray Dataset
reconstruction. Also override iterate_slices() to yield CSR row-batches
instead of relying on xarray's isel().

Move eliminate_zeros() to freeze time (from_mutable) so the cleanup
happens once rather than on every to_polars() call.

LP write is now 20-40% faster than master across all benchmark models.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@FBumann FBumann mentioned this pull request Mar 23, 2026
10 tasks
@coroa
Copy link
Member

coroa commented Mar 23, 2026

thank you

@FabianHofmann
Copy link
Collaborator

FabianHofmann commented Mar 23, 2026

this is already nice! I made some LP writing experiment in https://github.com/FabianHofmann/crs2lp which showed quite some improvements in performance. It directly writes the LP file from the CRS objects using custom rust module.

That said, I am happy to pull this one in to keep changes small but in the long term it is probably worth to move the implementation in the repo.

@FabianHofmann FabianHofmann marked this pull request as ready for review March 23, 2026 20:24
@FabianHofmann
Copy link
Collaborator

@coroa this is great, one big question that came up when looking at the code is: why renaming Constraint to MutableConstraint and making Constraint the frozen one, and not instead keep Constraint and add a FrozenConstraint? From a user perspective the current renaming is breaking and the exposed object has a non-intuitive name.

@FabianHofmann
Copy link
Collaborator

@coroa this is great, one big question that came up when looking at the code is: why renaming Constraint to MutableConstraint and making Constraint the frozen one, and not instead keep Constraint and add a FrozenConstraint? From a user perspective the current renaming is breaking and the exposed object has a non-intuitive name.

this should be in the other pr, moving it there (sorry)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants