Skip to content

feat: Autogenerate sparse matrix constructor functions with type-safe conversions#2532

Merged
krlmlr merged 19 commits intomainfrom
copilot/autogenerate-non-callback-functions
Jan 24, 2026
Merged

feat: Autogenerate sparse matrix constructor functions with type-safe conversions#2532
krlmlr merged 19 commits intomainfrom
copilot/autogenerate-non-callback-functions

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 24, 2026

Description

Enables autogeneration for igraph_sparse_adjacency, igraph_sparse_weighted_adjacency, and igraph_weighted_sparsemat by implementing the missing R-to-C SPARSEMAT type converter with proper type handling and memory management.

Changes

  • Implemented Rz_SEXP_to_sparsemat() in src/rinterface_extra.c

    • Converts Matrix package dgCMatrix to igraph_sparsemat_t
    • Handles both integer and double value types with separate code paths (no coercion)
    • Compresses from triplet to column-compressed format
    • Throws clean error messages for invalid types
  • Updated Rz_SEXP_to_vector_int_copy() in src/rinterface_extra.c

    • Now handles both INTSXP and REALSXP types without coercion
    • Uses separate code paths for integer and real inputs
    • Provides clear error messages for unsupported types
  • Fixed SPARSEMAT type handling in tools/stimulus/types-RC.yaml

    • Added proper IGRAPH_FINALLY registration for IN SPARSEMAT parameters
    • Added explicit cleanup with igraph_sparsemat_destroy in OUTCONV
    • Prevents memory leaks and "corrupt finally stack" errors
  • Enabled autogeneration for three sparse matrix functions

    • Removed IGNORE: RR, RC flags from tools/stimulus/functions-R.yaml
    • Changed INOUT to IN for sparse adjacency functions (don't return modified matrix)
    • Regenerated R/aaa-auto.R and src/rinterface.c
  • Fixed pre-existing bug in igraph_read_graph_ncol and igraph_read_graph_lgl

    • Added missing OUT GRAPH graph parameter to PARAMS overrides
  • Updated tests in tests/testthat/test-aaa-auto.R

    • Fixed tests to use symmetric matrices for undirected graphs
    • Corrected error test expectations (regular matrices auto-convert to sparse)
    • Removed tracked _problems test files (now in .gitignore)
    • All 1247 tests now pass

Example

library(Matrix)
M <- sparseMatrix(i = c(1, 2, 3, 4), j = c(2, 3, 4, 1), x = 1, dims = c(4, 4))
g <- sparse_adjacency_impl(adjmatrix = M, mode = "directed", loops = "once")
  • By submitting this pull request, I assign the copyright of my contribution to The igraph development team.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits January 24, 2026 10:06
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
…ions

Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Copilot AI changed the title [WIP] Autogenerate all non-callback non-SPARSEMAT functions feat: Autogenerate sparse matrix constructor functions Jan 24, 2026
Copilot AI requested a review from krlmlr January 24, 2026 10:19
@krlmlr krlmlr marked this pull request as ready for review January 24, 2026 11:13
Copilot AI review requested due to automatic review settings January 24, 2026 11:13
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Contributor

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot: Also fix test failures.

Comment thread src/rinterface_extra.c Outdated
int need_unprotect = 0;
if (TYPEOF(x_slot) == INTSXP) {
/* Convert integers to reals */
x_slot = PROTECT(Rf_coerceVector(x_slot, REALSXP));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Write code for integers and reals, avoid coercion. Throw a clean error if not integer or real.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in aa65ef4. Both Rz_SEXP_to_sparsemat and Rz_SEXP_to_vector_int_copy now handle integer and real types with separate code paths, avoiding coercion. Clean error messages are provided for invalid types.

However, tests still fail due to a separate Stimulus code generation issue: the autogenerated code for INOUT SPARSEMAT parameters has a mismatched IGRAPH_FINALLY_CLEAN(1) without a corresponding IGRAPH_FINALLY registration, causing "corrupt finally stack" errors. This needs to be fixed in the Stimulus code generator.

Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 24, 2026

@copilot: Also fix test failures.

The test failures are caused by a Stimulus code generation bug for INOUT SPARSEMAT parameters. The autogenerated code in src/rinterface.c lines 689-724 has an extra IGRAPH_FINALLY_CLEAN(1) at line 714 without a corresponding IGRAPH_FINALLY registration for c_adjmatrix after it's created at line 699. This causes "corrupt finally stack" errors.

The conversion functions have been fixed to properly handle integer and real types without coercion (commit aa65ef4), but the test failures require fixing the Stimulus template that generates code for INOUT SPARSEMAT parameters.

Copilot AI changed the title feat: Autogenerate sparse matrix constructor functions feat: Autogenerate sparse matrix constructor functions with type-safe conversions Jan 24, 2026
Copilot AI requested a review from krlmlr January 24, 2026 17:27
Copy link
Copy Markdown
Contributor

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot: Also fix tests:

── Error ('test-aaa-auto.R:11145:3'): sparse_adjacency_impl basic ──────────────
Error: Error in `sparse_adjacency_impl(adjmatrix = M)`: At vendor/cigraph/src/core/error.c:335 : Corrupt finally stack: trying to pop 1 element(s) when only 0 left. This is an unexpected igraph error; please report this as a bug, along with the steps to reproduce it.
Please restart your R session to avoid crashes or other surprising behavior.
Backtrace:
    ▆
 1. └─igraph:::sparse_adjacency_impl(adjmatrix = M)
── Error ('test-aaa-auto.R:11192:3'): sparse_weighted_adjacency_impl basic ─────
Error: Error in `sparse_weighted_adjacency_impl(adjmatrix = M)`: At vendor/cigraph/src/core/error.c:335 : Corrupt finally stack: trying to pop 1 element(s) when only 0 left. This is an unexpected igraph error; please report this as a bug, along with the steps to reproduce it.
Please restart your R session to avoid crashes or other surprising behavior.
Backtrace:
    ▆
 1. └─igraph:::sparse_weighted_adjacency_impl(adjmatrix = M)
── Failure ('test-aaa-auto.R:11262:3'): weighted_sparsemat_impl errors ─────────
Expected `weighted_sparsemat_impl(...)` to throw a error.

[ FAIL 3 | WARN 0 | SKIP 4 | PASS 7519 ]

@@ -0,0 +1,21 @@
# Extracted from test-aaa-auto.R:11145
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Purge these files, they are now ignored.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in 0cd85d9. The _problems files have been purged from the repository.

Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 24, 2026

@copilot: Also fix tests:

...

Fixed in 0cd85d9. The "corrupt finally stack" errors were caused by missing IGRAPH_FINALLY registration for IN SPARSEMAT parameters. Updated the type definition to properly register and destroy sparse matrices. All tests now pass (1247 passed, 0 failures).

Copilot AI requested a review from krlmlr January 24, 2026 18:47
@krlmlr krlmlr enabled auto-merge (squash) January 24, 2026 19:52
@krlmlr krlmlr merged commit 2187107 into main Jan 24, 2026
5 of 6 checks passed
@krlmlr krlmlr deleted the copilot/autogenerate-non-callback-functions branch January 24, 2026 20:10
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