Skip to content

Clear all n_rowsA entries in gmt_matrix_vector_mult - #9254

Merged
Esteban82 merged 1 commit into
masterfrom
fix-matrix-vector-mult-memset
Oct 11, 2026
Merged

Esteban82 merged 1 commit into
masterfrom
fix-matrix-vector-mult-memset

Conversation

@Esteban82

Copy link
Copy Markdown
Member

gmt_matrix_vector_mult cleared n_colsA entries of the output vector instead of n_rowsA.

In builds without LAPACK this left the last two values of the smoothing spline (sample1d -Fs) wrong, which is why GMT_splines, smooth and smooth_deriv fail only in the Windows CI.

Written with Claude Sonnet 5.5, reviewed with Claude Opus 5.5.

The output vector c has n_rowsA entries but the function cleared only
n_colsA of them. In gmtlib_smooth_spline the call Q = K*c has
n_rowsA = n and n_colsA = n-2, so the last two entries of Q were left
holding the previous matrix values and s[n-2] and s[n-1] came out wrong.
Builds with LAPACK clear the output in gmt_matrix_matrix_mult first and
hide the problem, so it only showed up in builds without LAPACK (the
Windows CI), as failures of GMT_splines, smooth and smooth_deriv.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@Esteban82
Esteban82 requested review from joa-quim and seisman October 9, 2026 20:27
@Esteban82 Esteban82 added bug Something isn't working add-changelog Add PR to the changelog labels Oct 9, 2026
@joa-quim

joa-quim commented Oct 9, 2026

Copy link
Copy Markdown
Member

Again, not sure it's acceptable. I never had that problem in my local build but it's also true that I am using LAPACK

@seisman

seisman commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

I think this is correct.

The gmt_matrix_vector_mult function is for matrix-vector multiplication $A\mathbf{b} = \mathbf{c}$.

An $m \times n$ matrix $A$ multiplies an $n \times 1$ column vector $\mathbf{b}$ should produce an $m \times 1$ result vector $\mathbf{c}$. I.e., the resulting vector $c$ should have $m$ elements (the n_rowsA variable in the C code).

It can also be verified by codes near line 1347:

			c[row] += A[ij] * b[col];

in which row is 0 - n_rowsA.

@Esteban82

Copy link
Copy Markdown
Member Author

Again, not sure it's acceptable. I never had that problem in my local build but it's also true that I am using LAPACK

Indeed, with LAPACK the bug is hidden: gmt_matrix_matrix_mult clears the whole output before calling this function, so the memset inside never matters. Without LAPACK nobody does, and the last two values of the smoothing spline stay dirty.

As a check, the Windows CI (which builds without LAPACK) now passes the 3 tests that were failing because of this: GMT_splines, smooth and smooth_deriv (https://github.com/GenericMappingTools/gmt/actions/runs/37987086526).

@Esteban82
Esteban82 merged commit dcf615f into master Oct 11, 2026
17 of 20 checks passed
@Esteban82
Esteban82 deleted the fix-matrix-vector-mult-memset branch October 11, 2026 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

add-changelog Add PR to the changelog bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants