Skip to content

Fix matrix index slice#9

Merged
SteveDiamond merged 5 commits into
cvxpy:mainfrom
haozhu10015:fix-matrix-index-slice
May 24, 2026
Merged

Fix matrix index slice#9
SteveDiamond merged 5 commits into
cvxpy:mainfrom
haozhu10015:fix-matrix-index-slice

Conversation

@haozhu10015
Copy link
Copy Markdown
Contributor

This PR fixes #2 by updating matrix indexing/slicing semantics and adding a general matrix selection API.

Changes

  • Fix index(&X, i) on matrix expressions to mean X[i, :].
  • Fix slice(&X, start, stop) on matrix expressions to mean X[start:stop, :].
  • Add AxisIndex and select(&expr, rows, cols) for general 2D selection:
    • row selection
    • column selection
    • scalar element selection
    • rectangular block selection
  • Add indexc(&X, j) and slicec(&X, start, stop) aliases for column indexing/slicing.
  • Add early bounds validation for invalid scalar/vector/matrix selection.
  • Update canonicalization and evaluation so matrix selections use column-major indexing consistently.

Note that in the current implementation, integer indexing will drop the selected axis. For example, index(&X, 1) on a matrix X with shape (3, 4) returns a vector with shape (4,), while slice(&X, 1, 2) preserves the axis and returns a matrix with shape (1, 4).

@SteveDiamond
Copy link
Copy Markdown
Collaborator

Is this the same semantics as CVXPY and NumPy? Or is there a matrix library in rust we should be aligning with?

@haozhu10015
Copy link
Copy Markdown
Contributor Author

This was intended to match CVXPY/NumPy semantics. We could instead align the public API with nalgebra’s Rust semantics, but that would be a different indexing model from the current implementation, since, for example, linear indexing follows column-major storage order. I personally prefer the CVXPY semantics, but I’m not sure which convention would be preferable for most cvxrust users.

@SteveDiamond
Copy link
Copy Markdown
Collaborator

Ok thanks for clarifying. I think this approach is fine for now, we can revisit later if necessary.

@SteveDiamond SteveDiamond merged commit 92da978 into cvxpy:main May 24, 2026
10 checks passed
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.

Slices and index giving unintuitive results

2 participants