Skip to content
This repository has been archived by the owner on Oct 23, 2020. It is now read-only.

Too aggressive default optimization? #1321

Open
akturner opened this issue May 11, 2017 · 12 comments
Open

Too aggressive default optimization? #1321

akturner opened this issue May 11, 2017 · 12 comments
Assignees

Comments

@akturner
Copy link
Contributor

The sea ice core has had for a while denormalization and underflow warnings. I have located the places these warning are generated (in the column physics package) and fixed them. In doing so I discovered some disturbing behaviour. A underflow warning remains but only with -O3 optimization, not -O[0-2]. Also putting print statements around the fixed code regions removes the warnings for -O3. I'm concerned that using -O3 as our default optimization is too aggressive. This level of optimization is often described as unsafe and I wonder whether we should change to -O2.

@mgduda
Copy link
Contributor

mgduda commented May 11, 2017

Underflow warnings may not necessarily be cause for concern, though I suppose it depends on where they occur in the code. I'll add this as a topic for discussion on the 15 May telecon.

@akturner
Copy link
Contributor Author

Thanks Michael, I was going to add it as well. My bigger concern was the change with optimization number. I've seen -O3 described as unsafe, which also is cause potentially for concern.

@philipwjones
Copy link
Contributor

Guys, the description of -O3 as unsafe is kinda like a consumer warning generally and more of a CYA for the compiler vendor. I have found instances of too-aggressive optimizations very rare and compiler vendors normally include those sorts of optimizations as separate flags for those who feel they can trade accuracy for performance. Reducing to -O2 usually leaves too much performance on the table, so would not advocate for a general reduction. @akturner , I'm happy to look at the routine is question and see if I can suss out the problem and see if a coding/algorithm modification can fix. In the worst case, we can reduce the optimization level for that routine only if it truly is causing a problem.

@akturner
Copy link
Contributor Author

There are the lines causing the initial denorm and underflow warnings.
https://github.com/MPAS-Dev/MPAS/blob/cice/develop/src/core_cice/column/ice_shortwave.F90#L3267
https://github.com/MPAS-Dev/MPAS/blob/cice/develop/src/core_cice/column/ice_shortwave.F90#L3276
https://github.com/MPAS-Dev/MPAS/blob/cice/develop/src/core_cice/column/ice_shortwave.F90#L3297
Fixing these leaves a underflow warning only for -O3. Inserting print statements can remove the warning making debugging difficult. @philipwjones If you can find where the remaining error is, that would be grand!

@philipwjones
Copy link
Contributor

All of these have the form:
result = max(exp_min, exp(-arg_expr))

This will still lead to underflows in the intermediate exp evaluation that are nominally replaced by exp_min. This leaves you at the mercy of the compiler's implementation/management the underflow itself. A more robust approach would be to limit the argument to exp, e.g.:
arg = min(arg_max, arg_expr)
result = exp(-arg)
where you can choose arg_max such that exp(-arg_max) would give you something like the exp_min in the original. That way you avoid the underflow altogether and are not at the mercy of the compiler implementation or optimization level.

@akturner
Copy link
Contributor Author

akturner commented May 11, 2017

@philipwjones : Yes, thats what I've done. Yet still an underflow warning persists just with -O3. And goes away with print statement around your suggested modifications.

See here for my corrections: https://github.com/akturner/MPAS/tree/cice/ieee_exceptions

@philipwjones
Copy link
Contributor

@akturner - sorry I misunderstood - thought the lines you were pointing me to were the fixes. For which compiler/machine is this happening? I'll see if it's an actual bug or whether they're just trying to avoid a memory op. In the meantime, you can try the confuse-a-compiler trick and reorder the statements so that tmpexp isn't adjacent to the exp evaluation. Or maybe see whether it's ts that's getting large (and limit that) or whether it's the other factors that are pushing it too large.

@akturner
Copy link
Contributor Author

@philipwjones: No worries - don't think I was clear! This is happening on OSX and with gfortran. I will try your suggestions.

@mgduda
Copy link
Contributor

mgduda commented May 16, 2017

@akturner With gfortran, you can try using the flags -O3 -g -fbacktrace -ffpe-trap=underflow to identify the lines where the underflows are occurring when print statements are removed.

@akturner
Copy link
Contributor Author

@mgduda: Thanks!

@akturner
Copy link
Contributor Author

@mgduda: These options didnt produce a useful stack trace:

pn1512828:rundir akt$ cice_model

Program received signal SIGFPE: Floating-point exception - erroneous arithmetic operation.

Backtrace for this error:
#0 0x104256e66
#1 0x10425662c
#2 0x7fff84ec1f19
Floating point exception: 8

@matthewhoffman
Copy link
Member

@akturner , try it on Wolf. Or you can try running within lldb on your mac like lldb cice_model then run.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants