Skip to content
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.

Sign up for GitHub

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

Disable FP checks for a certain LAPACK routine. #1268

Closed
wants to merge 1 commit into from
Closed

Disable FP checks for a certain LAPACK routine. #1268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cime/cime_config/acme/machines/config_compilers.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ for mct, etc.
<!--ADD_SLIBS> -mkl -lpthread -lm </ADD_SLIBS-->

<ADD_GPTL_CPPDEFS> -DHAVE_PAPI </ADD_GPTL_CPPDEFS>
<ADD_CPPDEFS> -DHAVE_SLASHPROC </ADD_CPPDEFS>
<ADD_CPPDEFS> -DARCH_MIC_KNL -DHAVE_SLASHPROC </ADD_CPPDEFS>
<!--ADD_CPPDEFS> -DHAVE_NANOTIME -DBIT64 -DHAVE_VPRINTF -DHAVE_BACKTRACE -DHAVE_SLASHPROC -DHAVE_COMM_F2C -DHAVE_TIMES -DHAVE_GETTIMEOFDAY </ADD_CPPDEFS> -->
<MPIFC> ftn </MPIFC>
<MPICC> cc </MPICC>
Expand Down
2 changes: 1 addition & 1 deletion components/cam/bld/configure
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2024,7 +2024,7 @@ if ($cosp) { $cfg_cppdefs .= ' -DUSE_COSP'; }
if ($clubb_sgs == 1) {
$cfg_cppdefs .= ' -DCLUBB_SGS';
$cfg_cppdefs .= ' -DCLUBB_CAM';
$cfg_cppdefs .= ' -DNO_LAPACK_ISNAN';
#$cfg_cppdefs .= ' -DNO_LAPACK_ISNAN';
Copy link
Contributor

Choose a reason for hiding this comment

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

@ndkeen : Would you please let me know why it is necessary to comment out this CPP directive? I may have missed it while you were discussing this issue in issue #1183

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not necessary. This macro addresses an old issue where LAPACK may not have had the isnan() function, which they probably all have now. I don't think we need it. If you would like me to take it out of the PR, I can do that. However, I think it is more efficient to clean-up things like this as we go.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. If you think it is no longer required, please remove this altogether (rather than commenting out), otherwise all these commented out lines will stay in the code forever.

$cfg_cppdefs .= " -DCLUBB_REAL_TYPE=dp";
}

Expand Down
24 changes: 22 additions & 2 deletions components/cam/src/physics/clubb/lapack_wrap.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,12 @@ subroutine tridag_solve &

use clubb_precision, only: &
core_rknd ! Variable(s)

#ifndef NDEBUG
#ifdef ARCH_MIC_KNL
use, intrinsic :: ieee_exceptions
!use, intrinsic :: ieee_arithmetic
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove this commented out line if it is not required to keep it for future reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can remove the comment. Are we no longer fans of comments?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to keep the descriptive comments you added about why these line are added. I was just talking about removing the line !use, intrinsic :: ieee_arithmetic for the same reason I mentioned above. From your comment below it seems like it might be useful in the future if for some reason use, intrinsic :: ieee_exceptions doesn't work for a machine/compiler. So please keep it and do not make any changes to this file.

#endif
#endif
implicit none

! External
Expand Down Expand Up @@ -265,9 +270,24 @@ subroutine tridag_solve &
! SUBROUTINE DGTSV( N, NRHS, DL, D, DU, B, LDB, INFO )
!-----------------------------------------------------------------------


if ( kind( diag(1) ) == dp ) then
call dgtsv( ndim, nrhs, subd(2:ndim), diag, supd(1:ndim-1), &
#ifndef NDEBUG
#ifdef ARCH_MIC_KNL
! when floating-point exceptions are turned on, this call was failing with a div-by-zero on KNL with Intel/MKL. Solution
! was to turn off exceptions only here at this call (and only for machine with ARCH_MIC_KNL defined) (github 1183)
call ieee_set_halting_mode(IEEE_DIVIDE_BY_ZERO, .false.) ! Turn off stopping on div-by-zero only
!call ieee_set_halting_mode(ieee_usual, .false.) ! Turn off stopping on all floating-point exceptions
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, can we remove the commented out lines (except the descriptive comments) unless they are necessary to keep?

Otherwise, it all looks good to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can remove comments. There are two different ways to turn on/off floating-point issues and I thought that some machines/compilers might have ieee_exceptions and not the other or vice/versa.

Copy link
Contributor

Choose a reason for hiding this comment

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

From your explanation above, I think it makes sense to keep these lines for future reference.

#endif
#endif
call dgtsv( ndim, nrhs, subd(2:ndim), diag, supd(1:ndim-1), &
rhs, ndim, info )
#ifndef NDEBUG
#ifdef ARCH_MIC_KNL
call ieee_set_halting_mode(IEEE_DIVIDE_BY_ZERO, .true.) ! Turn back on stopping on div-by-zero only
!call ieee_set_halting_mode(ieee_usual, .true.) ! Turn off stopping on all floating-point exceptions
#endif
#endif

else if ( kind( diag(1) ) == sp ) then
call sgtsv( ndim, nrhs, subd(2:ndim), diag, supd(1:ndim-1), &
Expand Down