Skip to content

Updates the documentation of xGEMV and xGBMV related to when M=0 and N=0#843

Merged
langou merged 2 commits intoReference-LAPACK:masterfrom
weslleyspereira:fix-248
Aug 22, 2023
Merged

Updates the documentation of xGEMV and xGBMV related to when M=0 and N=0#843
langou merged 2 commits intoReference-LAPACK:masterfrom
weslleyspereira:fix-248

Conversation

@weslleyspereira
Copy link
Copy Markdown
Collaborator

@weslleyspereira weslleyspereira commented May 26, 2023 •
edited
Loading

Closes #248.
Closes #788.

The documentation of GEMV (and GBMV) misses a boundary case, namely when M=0 or N=0. There are two possibilities for the solution of this issue:

  1. The behavior of GEMV (and GBMV) does not seem to match the behavior of GEMM, SYRK, SYR2K, HERK and HER2K regarding matrices with zero input sizes. In GEMM, for instance, C gets updated even if the internal dimension K is zero. In GEMV (and GBMV), if the internal dimension (M or N) is zero, Y does not get updated. The first solution is, therefore, to change the condition in GEMV (and GBMV) to be compatible to the lv3 BLAS.

  2. The behavior of GEMV (and GBMV) is aligned to its original proposal (https://dl.acm.org/doi/10.1145/42288.42291) that says:
    Note that it is permissible to call the routines with M or N = 0, in which case the routines exit immediately without referencing their vector or matrix arguments.
    Moreover, tests in (testBLAS) confirm that several BLAS implementations conform with this rule.

This PR proposes a fix for the issue using (2), which preserves the current behavior and is compatible to several BLAS implementations.

@codecov
Copy link
Copy Markdown

codecov bot commented May 26, 2023 •
edited
Loading

Codecov Report

Patch and project coverage have no change.

Comparison is base (de2b976) 0.00% compared to head (f59ece6) 0.00%.
Report is 92 commits behind head on master.

❗ Current head f59ece6 differs from pull request most recent head 658ac9d. Consider uploading reports for the commit 658ac9d to get more accurate results

Additional details and impacted files 
@@            Coverage Diff            @@
##           master     #843     +/-   ##
=========================================
  Coverage    0.00%    0.00%             
=========================================
  Files        1908     1918     +10     
  Lines      186962   188614   +1652     
=========================================
- Misses     186962   188614   +1652

see 146 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@angsch
Copy link
Copy Markdown
Collaborator

angsch commented Jun 13, 2023

Is this behaviour checked/enforced by some test? It is a subtle change that could slip through when vendors upgrade their LAPACK version and their implementation does not follow reference-LAPACK.

@weslleyspereira
Copy link
Copy Markdown
Collaborator Author

weslleyspereira commented Aug 22, 2023

The most recent commit also updates *la_gbamv.f and *la_geamv.f using strategy (2).

We have:

  1. routines with m-by-n A and y: *gemv.f, *gbmv.f, *la_gbamv.f, *la_geamv.f. (8 + 4 + 4 + 4 =) 20 files. Documentation was updated.
  2. routines with n-by-n A and y: *symv.f, *spmv.f, *sbmv.f, *hemv.f, *hpmv.f, *hbmv.f, *la_syamv.f, *la_heamv.f. (4 + 4 + 6 + 2 + 2 + 2 + 4 + 2 =) 26 files. No need to update documentation.
  3. routines with m-by-n A and no y: *tbmv.f, *trmv.f, *tpmv.f. (4 + 4 + 4 =) 12 files. No need to update documentation.

@weslleyspereira
Copy link
Copy Markdown
Collaborator Author

weslleyspereira commented Aug 22, 2023

Is this behaviour checked/enforced by some test? It is a subtle change that could slip through when vendors upgrade their LAPACK version and their implementation does not follow reference-LAPACK

Thanks for asking! Yes, we currently check this behavior in (testBLAS). In fact, multiple BLAS implementations (all libraries I tested so far in testBLAS) satisfy this behavior. We do not have such checks in the LAPACK test suite currently.

@langou langou merged commit 2983298 into Reference-LAPACK:master Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@martin-frbg martin-frbg martin-frbg approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Y is misscaled in ?GEMV for some cases gbmv leaves result uninitialized for zero column matrix

4 participants

@weslleyspereira @angsch @martin-frbg @langou