Skip to content

Remove legacy function LABAD #805

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 22, 2023
Merged

Remove legacy function LABAD #805

merged 2 commits into from
Mar 22, 2023

Conversation

angsch
Copy link
Collaborator

@angsch angsch commented Mar 21, 2023

Description

LABAD is a legacy function that used to make LAPACK usable on machines that did not comply with IEEE arithmetic. This PR removes the function calls to LABAD and turns LABAD itself into a no-op. See also #96.

If you agree that the removal (rather than turning the routine only into a no-op) is the right approach, I would go over the other routines (testing etc.), too.

Checklist

  • The documentation has been updated.
  • If the PR solves a specific issue, it is set to be closed on merge.

@codecov
Copy link

codecov bot commented Mar 21, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (cfaa5ae) 0.00% compared to head (1901f7d) 0.00%.

❗ Current head 1901f7d differs from pull request most recent head 542f4f8. Consider uploading reports for the commit 542f4f8 to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##           master     #805    +/-   ##
========================================
  Coverage    0.00%    0.00%            
========================================
  Files        1908     1908            
  Lines      187104   186972   -132     
========================================
+ Misses     187104   186972   -132     
Impacted Files Coverage Δ
SRC/cgees.f 0.00% <ø> (ø)
SRC/cgeesx.f 0.00% <ø> (ø)
SRC/cgeev.f 0.00% <ø> (ø)
SRC/cgeevx.f 0.00% <ø> (ø)
SRC/cgels.f 0.00% <ø> (ø)
SRC/cgelsd.f 0.00% <ø> (ø)
SRC/cgelss.f 0.00% <ø> (ø)
SRC/cgelst.f 0.00% <ø> (ø)
SRC/cgelsy.f 0.00% <ø> (ø)
SRC/cgesc2.f 0.00% <ø> (ø)
... and 118 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@weslleyspereira
Copy link
Collaborator

Great PR! Thanks a lot for working on that.

I think we have at least 3 other people in favor of removing LABAD from LAPACK. Please, see #727.

langou
langou previously approved these changes Mar 21, 2023
@langou
Copy link
Contributor

langou commented Mar 21, 2023

Hi,

I am in favor of this PR, and in favor of removing LABAD out of the library.

Julien.

@martin-frbg
Copy link
Collaborator

How about adding a (non-fatal !) build-time test just in case there is something more modern around than a late-1990s Cray with the same limitations ?

@langou
Copy link
Contributor

langou commented Mar 21, 2023

Thanks @martin-frbg.

I think I understand what you are saying but (1) I do not think LABAD translate to any modern architecture, and ever will, (2) it was a dirty fix back then, it is still a dirty fix and we should remove this, if we want to do this again, we should do something differently.

The way I am reading LABAD is as follows.

(1) We can only use half of the range of numbers with confidence on old Cray's machine. So we want to restrict the range on an old Cray's by doing something like:

lapack/SRC/dlabad.f

Lines 94 to 95 in cfaa5ae

SMALL = SQRT( SMALL )
LARGE = SQRT( LARGE )

(2) How do we know we are on an old Cray? They have a huge range of number, LARGE is greater than +10^2000, so we can know we are on an old Cray by checking LARGE:

IF( LOG10( LARGE ).GT.2000.D0 ) THEN

The whole thing feels bogus to me. This is nice historical piece of software but I think we can safely move on from this routine.

Julien.

@angsch
Copy link
Collaborator Author

angsch commented Mar 22, 2023

LABAD is now fully removed from the numerical functions and tests; LABAD itself is no-op.

I intentionally did not move LABAD to the deprecated folder. This is to avoid an undefined symbol if someone calls LABAD in a custom external legacy routine. Recall that the deprecated routines are not built by default.

@angsch angsch marked this pull request as ready for review March 22, 2023 08:34
@langou langou merged commit 7ac31fa into Reference-LAPACK:master Mar 22, 2023
@langou
Copy link
Contributor

langou commented Mar 22, 2023

Thanks a lot @angsch. This is a long overdue PR and this is great to have this finally done. Thanks a lot.

Leaving LABAD in SRC as a no-op is fine with me. I think it is fine to move it to DEPRECATED too but there is not rush in doing so.

Maybe we should add in LABAD, in the section "Purpose",

Deprecated: As of March 2023, this routine is a no-op and is not used in any LAPACK subroutines, (neither in SRC nor in TESTING,) this routine might be remove from the package in the near future.

Something like that.

@angsch angsch deleted the labad branch March 23, 2023 06:33
@weslleyspereira weslleyspereira mentioned this pull request May 23, 2023
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.

4 participants