Closed A-CGray closed 2 months ago
@eirikurj @A-CGray Here are the details I've uncovered:
src/projections.F90
uses an uppercase 'F' extension and requires compiler pre-processing. This affects the line search because the line search parameters are defined as global constants using a #define
statement:
! Define some parameters used for all projection functions
#define LSFailMax 2
#define wolfe .001
#define nLine 20
Our current compiler setup with ifx
doesn't support the necessary pre-processing. I found this thread that somewhat explains the basics of the issue. This thread may also be relevant w.r.t compiler flags for pre-processing.
Either A) We figure out the correct compiler setup for the pre-processing or B) We slightly modify the projections so they don't require pre-processing. Let me know what you think.
Nice, thanks for finding this @lamkina . If this is as simple as changing what compiler flags are used to enable pre-processing, and the same flags will work for both ifort and ifx (and gcc?) then I'm fine with just doing that. Otherwise, maybe just switch to defining these as global constants so that we don't need to have another thing depend on the type of compiler we're using (I'm only familiar with doing this in C++ where I'd write something like const int LS_FAIL_MAX = 2;
, but I assume you can do something similar in Fortran).
Also, I see adtProjections also has an F90 extension, but I didn't see any preprocessing commands in there, any idea what's going on there?
I think the cleaner solution is to define these as constants and have all the extensions be lowercase f90
.
I don't think the adtProjections
need to be F90
so we can test that as part of this fix.
I'll wait for @eirikurj to comment then we can decide on the right way to fix this.
I took a look, and I am not convinced that this is a preprocessing issue. To explicitly enable the preprocessor we can include the flag -fpp
. This however has no effect and tests still fail. Even removing the define
statements by manually search-replace the variables with values should alleviate this issue. However, it does not seem to have any effect as the tests fail.
Disabling compile optimization, i.e., setting -O0
, will make the tests pass on an unmodified code, i.e., with the define
statements (no -fpp
flag used here). Any other optimization setting will make the tests fail. This suggests that optimization is changing the behavior, resulting in a different branch/path being taken (based on values) on the optimized code. Adding the flag -fp-model=precise
, disables some optimizations making the floating point comparison more strict and value-safe, and with this option all tests pass regardless of optimization level. I briefly tested this with ifort
and the behavior seems to be more inline with what gfortran
behavior. Adding the same flag to ifort
makes the behavior consistent with ifx
using the same flag.
In code, the divergence between ifx
and gfortran
happens here https://github.com/mdolab/pyspline/blob/b488ed5742edd7dea5c0d2f133974f30b9889e64/src/projections.F90#L153
Running one of the test/points locally, the state of the values are shown below,
Variable | ifx | gfortran |
---|---|---|
fval | 0.973086514811139 | 0.97308651481113850 |
nfval | 0.973086514811139 | 0.97308651481113850 |
pgrad | -4.045624659884222E-021 | -4.0456103257696632E-021 |
step | 1.00000000000000 | 1.00000000000000 |
c | 0.000000000000000E+000 | 4.0456103257696632E-021 |
The inputs nfval,fval,step
are the equal. pgrad
is not equal, but it effectively zero (below machine zero).
For ifx
since c
is zero, the subsequent step
is Infinty
, effectively breaking the remainder of the iterations, resulting in the test failing. For gfortran
the value for c
is some very small number. Setting -fp-model=precise
for ifx
will result in the same value final value as gfortran
. However, even if the final result is the same, the path is still not the same. ifx
will continue the iteration in the lineLoop
until hitting the maximum number of iterations, but not improve on the point. gfortran
and ifort
both exit the lineLoop
earlier, but it seems to be due to inexact numerics.
Aside from compiler tweaks, I think the issue (at least in part) is due to sufficient decrease of the Wolfe condition (see here https://github.com/mdolab/pyspline/blob/b488ed5742edd7dea5c0d2f133974f30b9889e64/src/projections.F90#L139), since technically it should include equals as well, i.e., be a <=
comparison, not <
. Adding this correction to the if
statement thus terminates the loop before this becomes an issue (since the gradient is zero), and the behavior is the same across all compilers ifx
, gfortran
, and ifort
. Granted, this is a small change in behavior, but I think practically there should be no change in final results, and it should avoid unnecessary iterations.
I dont think digging more into the compilers is worth it, aside from just adding the compiler flags I mention above, but feel as I think its important to study ifx
since its a new compiler for us. Moving the define
to a module is possible, but I dont think is needed. The primary thing is the stopping condition (sufficient decrease), which I have opened a PR for here https://github.com/mdolab/pyspline/pull/75.
Please test this carefully and let me know what you think.
Chiming in just to say that this is some incredible detective work! Updating the Wolfe condition here is a no-brainer. I wonder if we should look at other codes and see if there are "risky" if checks like this.
Wow @eirikurj this is a great find. I agree with your proposed fix in #75. On the preprocessing side, is -fpp
on by default?
@A-CGray yes preprocessing is automatically applied to a file if it has one of these extensions shown below (gfortran added for reference also).
Since we are using F90
for our source files, we should be good across all compilers we use.
Description
pyspline tests are failing on the latest intel docker images that use the
ifx
compiler, see test outputs hereCopied here for archiving
Steps to reproduce issue
Current behavior
…
Expected behavior
…
Code versions