Skip to content

Restore missing prototypes for deprecated LAPACK functions#367

Merged
langou merged 1 commit into
Reference-LAPACK:masterfrom
svillemot:master
Nov 23, 2019
Merged

Restore missing prototypes for deprecated LAPACK functions#367
langou merged 1 commit into
Reference-LAPACK:masterfrom
svillemot:master

Conversation

@svillemot

Copy link
Copy Markdown
Contributor

Some LAPACK functions prototypes were inadvertedly dropped in 3.9.0. As a
consequence, LAPACKE has several unresolved symbols.

Closes #365

Some LAPACK functions prototypes were inadvertedly dropped in 3.9.0. As a
consequence, LAPACKE has several unresolved symbols.

Closes Reference-LAPACK#365
@langou

langou commented Nov 23, 2019

Copy link
Copy Markdown
Contributor

Thanks @svillemot

@langou langou merged commit 3b4e807 into Reference-LAPACK:master Nov 23, 2019
@Reference-LAPACK Reference-LAPACK deleted a comment from codecov Bot Nov 23, 2019
@codecov

codecov Bot commented Nov 23, 2019

Copy link
Copy Markdown

Codecov Report

Merging #367 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #367   +/-   ##
=======================================
  Coverage   81.86%   81.86%           
=======================================
  Files        1863     1863           
  Lines      181008   181008           
=======================================
  Hits       148174   148174           
  Misses      32834    32834

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6acc99d...87536aa. Read the comment docs.

@mu578

mu578 commented Nov 23, 2019

Copy link
Copy Markdown

may you mark them deprecated either by moving them into a separate header lapack_deprecated.h or something like #define LAPACK_GLOBAL LAPACK_DEPRECATED ?

Comment thread LAPACKE/include/lapack.h
lapack_int* info );

#define LAPACK_sggsvd LAPACK_GLOBAL(sggsvd,SGGSVD)
lapack_int LAPACKE_sggsvd( int matrix_layout, char jobu, char jobv, char jobq,

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.

Shoudn't this be LAPACK_sggsvd instead of LAPACKE_sggsvd?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

there are two decl:
-Fortran callable from C
-The C plaster or the memory copies and transposes everywhere frontend.

@h-vetinari h-vetinari Apr 25, 2020

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.

I don't know the codebase well, but I presume the plaster is e.g. lapacke_sggsvd_work that does the transposing etc. while the actual C-to-Fortran wrapper is lapacke_sggsvd.

However, by that same argument, the group of [cdsz]geqpf-functions above should also be declared with LAPACKE_... but aren't (even though the have the same structure). From an outside POV, it would make sense that lapack.h refers to LAPACK-functions, not LAPACKE?

@mu578 mu578 Apr 25, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes surely (except the first paragraph which is plain wrong); however; it's a compatibility restore; was this way before; the best way would be to declare

#define LAPACK_DEPRECATED

LAPACK_DEPRECATED
#define LAPACK_dggsvd LAPACK_GLOBAL(dggsvd,DGGSVD)
lapack_int LAPACKE_dggsvd (int matrix_layout, char jobu, char jobv, char jobq, lapack_int m, lapack_int n, lapack_int p, lapack_int *k, lapack_int *l, double *a, lapack_int lda, double *b, lapack_int ldb, double *alpha, double *beta, double *u, lapack_int ldu, double *v, lapack_int ldv, double *q, lapack_int ldq, lapack_int *iwork);

for reference -> https://github.com/Reference-LAPACK/lapack/blob/master/LAPACKE/src/lapacke_sggsvd.c#L36

then calling its worker which finally calls the true Fortran-C binding (global intrinsic); was the original spaghetti design; not me-self; just iterating facts and being rational.

h-vetinari added a commit to h-vetinari/lapack that referenced this pull request Apr 27, 2020
…ons [cdsz]ggsv[dp]

These functions were inadvertently removed before 3.9.0 and readded
by Reference-LAPACK#367. However, some of the LAPACK-functions in lapack.h were
declared as LAPACKE-functions (which also have a different signature
than their LAPACK-counterparts), which leads (at least) to the compiler
emitting a [-Wimplicit-function-declaration] warning and not knowing
which function to choose:

[...]/LAPACKE/src/lapacke_cggsvp_work.c: In function 'LAPACKE_cggsvp_work':
[...]/LAPACKE/include/lapack.h:3761:37: warning: implicit declaration of function 'cggsvp_'; did you mean 'cggsvp3_'? [-Wimplicit-function-declaration]
 #define LAPACK_cggsvp LAPACK_GLOBAL(cggsvp,CGGSVP)
                                     ^
[...]/LAPACKE/include/lapacke_mangling.h:12:39: note: in definition of macro 'LAPACK_GLOBAL'
 #define LAPACK_GLOBAL(lcname,UCNAME)  lcname##_
                                       ^~~~~~
[...]/LAPACKE/src/lapacke_cggsvp_work.c:52:9: note: in expansion of macro 'LAPACK_cggsvp'
         LAPACK_cggsvp( &jobu, &jobv, &jobq, &m, &p, &n, a, &lda, b, &ldb, &tola,
         ^~~~~~~~~~~~~

Here, we align the declarations in lapack.h with their callsites in
the various LAPACKE_[cdsz]ggsv[dp]_work functions again.

For more discussion see
conda-forge/lapack-feedstock#32

Suggested-By: Isuru Fernando <isuruf@gmail.com>
christoph-conrads pushed a commit to christoph-conrads/lapack that referenced this pull request May 23, 2021
Restore missing prototypes for deprecated LAPACK functions
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.

Prototypes for several deprecated LAPACK functions missing from LAPACKE/include/lapack.h

5 participants