Skip to content

SRC: detect all-zero Y columns in GEDMD scaling#1243

Open
nakatamaho wants to merge 3 commits intoReference-LAPACK:masterfrom
nakatamaho:fix/gedmd-sccoly-zero-columns
Open

SRC: detect all-zero Y columns in GEDMD scaling#1243
nakatamaho wants to merge 3 commits intoReference-LAPACK:masterfrom
nakatamaho:fix/gedmd-sccoly-zero-columns

Conversation

@nakatamaho
Copy link
Copy Markdown
Contributor

This PR adds the missing all-zero column check to the JOBS='Y'
scaling path in the GEDMD routines.

In the existing implementation, the JOBS='S' / JOBS='C' path
normalizes the columns of X and counts zero columns while doing so.
If all columns of X are zero, the routine returns early with
INFO = -8, since X is the 8th argument.

The corresponding JOBS='Y' path normalizes the columns of Y, but it
did not count zero columns of Y and therefore did not return early
when all columns of Y were zero. This made the handling of the
SCCOLY path asymmetric with the existing SCCOLX path.

This PR updates the JOBS='Y' path to:

  • initialize the zero-column counter before scanning Y
  • increment the counter for each zero column of Y
  • return early when all columns of Y are zero
  • use INFO = -10, matching Y as the 10th argument
  • document this condition in the routine header comments

Updated routines:

  • SRC/sgedmd.f90
  • SRC/dgedmd.f90
  • SRC/cgedmd.f90
  • SRC/zgedmd.f90

This is intended to make the input validation and scaling behavior of
the JOBS='Y' path consistent with the existing JOBS='S' / JOBS='C'
path. It also avoids continuing the computation after detecting that
the requested Y-based column scaling cannot be performed because all
columns of Y are zero.

Add the missing all-zero column check to the JOBS='Y' scaling path in the GEDMD routines. The existing JOBS='S'/'C' path detects when all columns of X are zero and returns INFO=-8, but the corresponding JOBS='Y' path did not count zero columns of Y or return early when all Y columns are zero.

Return INFO=-10 for the all-zero Y case, matching Y as the 10th argument and the existing SCCOLY error handling. Update the routine documentation to describe this condition.
langou
langou previously approved these changes Apr 14, 2026
@langou
Copy link
Copy Markdown
Contributor

langou commented Apr 18, 2026

From: Zlatko Drmac
Date: Wednesday, April 15, 2026 at 9:47 AM

First a summary:

X and Y are data that define implicitly a matrix A such that A*X \approx Y. So, in principle the caller can input any X and Y.   

However, if scaling options are active, the subroutine uses the opportunity to check for some inconsistencies - like all X is zero, or some are NaNs. Having all X zero indeed suggests something is wrong with the data. This is again checked after the SVD of X - if largest singular value is zero, error code -8 is returned.

If scaling of Y is selected, detected NaNs will trigger an error message. If all Y are zero, this also suggests something is wrong with the data, although it may implicitly define a zero matrix - this again is something that the caller probably did not want.

So, this is something I call "a political decision" - the roles of X and Y are not symmetric in this case but one can simply define/declare that zero input (X or Y) will be reported as error. This is what is in the proposed correction. Perhaps it should be done. Otherwise, DGEEV will get a zero matrix on input. In DGEEV you have a quick return if N=0. On the other hand, to check for scaling DGEEV computes ANRM and scales if needed but it does not use the opportunity for quick return if ANRM=0.

So, I would say it is OK to return an error code if Y=0. The only "con" is that then the subroutine behaves differently if scaling of Y is not requested and Y=0

(For X=0, even without scaling, after the SVD X=0 will be detected and reported error.)

So, in the end, it is a "political decision" 😀

@langou
Copy link
Copy Markdown
Contributor

langou commented Apr 18, 2026

From: Zlatko Drmac
Date: Thursday, April 16, 2026 at 8:51 AM

It is indeed important to discuss all aspects of such "singularities". Well designed behaviour for singular cases may save the day in mission critical applications.  

If Y = 0 then the matrix whose eigenpairs were to be identified is zero matrix (based on the supplied data) so in principle the question is what an eigenvalue subroutine should return if zero matrix is given as input? If this is DGEEV then it should return eigenvalues and eigenvectors (which ones) of the zero matrix. In the DMD case we can say zero matrix is something unheard of and  return error code.  Making the change as suggested is OK, but also some another flag could be raised. E.g., note that DGEDMD uses positive values of INFO to return some diagnostics/warnings. So it would be also wise to update INFO to make sure the caller is well informed. 

@martin-frbg
Copy link
Copy Markdown
Collaborator

Framing this as a "political decision" makes me wonder if the change would break existing user code. Returning early if no useful work can be done is one thing, but erroring out in what I assume to be a benign case (continuing would not cause an exception or loop(?) ) would only put unexpected burden on users to check their inputs (making the added code in LAPACK redundant) or to catch the error.
INFO is already set to 1 when a quick return is made on void input, i.e. M or N zero, perhaps this could be used for the Y=0 case as well ? With the datum being informational, it also wouldn't matter as much that the check is done in one code path only.

Treat the all-zero Y case in the JOBS='Y' scaling path as a degenerate input diagnostic instead of an illegal argument.

Previously this path returned INFO = -10 and called XERBLA, which made the condition look like an invalid tenth argument. However, an all-zero Y matrix is not an illegal argument by itself; it is a degenerate data case detected under the JOBS='Y' scaling semantics.

Introduce INFO = 5 for this early-return condition and document it as a positive diagnostic code. The GEDMDQ wrappers now propagate INFO = 5 as a terminal diagnostic from the underlying GEDMD call.
@nakatamaho
Copy link
Copy Markdown
Contributor Author

Thanks, I agree that treating the all-zero Y case as INFO = -10 was too strong.

I have updated the change accordingly. The JOBS='Y' all-zero-Y case is now reported as INFO = 5, documented as a positive diagnostic, and no longer routed through XERBLA. The routine returns early with K = 0 in this case.

The motivation is to keep the explicit detection of the degenerate JOBS='Y' scaling case, while avoiding reclassifying it as an illegal 10th argument. In other words, this is now treated as a degenerate-input diagnostic rather than an argument error.

I also chose not to reuse INFO = 1, since that already denotes the void-input quick return (M = 0 or N = 0), and it seemed better to let callers distinguish empty input from the all-zero-Y case.

Finally, I updated the corresponding *GEDMDQ wrappers so that INFO = 5 is propagated as a terminal diagnostic from the underlying *GEDMD call.

Add regression coverage for the JOBS='Y' scaling path when all columns of Y are zero.

The test drivers now check that xGEDMD returns early with K = 0 and INFO = 5 for this degenerate input case, rather than treating Y as an illegal argument. The xGEDMDQ drivers also check that the same diagnostic is propagated from the underlying xGEDMD call.

Separate failure counters are used for the GEDMD and GEDMDQ diagnostic checks so the summaries report the affected routine clearly.
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