SPECFEM / specfem3d

SPECFEM3D_Cartesian simulates acoustic (fluid), elastic (solid), coupled acoustic/elastic, poroelastic or seismic wave propagation in any type of conforming mesh of hexahedra (structured or not).
GNU General Public License v3.0
412 stars 229 forks source link

Fails compile with xlf #818

Closed jlost closed 7 years ago

jlost commented 8 years ago

specfem3d fails compile with xlf. make fails at "src/shared/read_parameter_file.F90", line 412.40: 1515-009 (E) Null literal string is not permitted. A single blank is assumed. This can be worked around by manually setting -qhalt=s in the Makefile. Passing it into the FCFLAGS doesn't work. The desired end result of tweaking the code or Makefile is that specfem3d can build with xlf without workaround or error.

Note - The same problem exists in specfem2d: https://github.com/geodynamics/specfem2d/issues/602

komatits commented 8 years ago

Yes, I fixed it in the 2D code by removing all the empty strings in assignments or comparisons (i.e. things like '' or "" assigned to a variable) because xlf is right, that is not part of the Fortran standard.

However in 3D I did not see how to remove them everywhere without the risk of introducing side effects.

That is why it would be very important to add xlf support to BuildBot.

Thanks, Dimitri.

On 08/08/2016 14:15, James wrote:

specfem3d fails compile with xlf. make fails at |"src/shared/read_parameter_file.F90", line 412.40: 1515-009 (E) Null literal string is not permitted. A single blank is assumed.| This can be worked around by manually setting -qhalt=s in the Makefile. Passing it into the FCFLAGS doesn't work. The desired end result of tweaking the code or Makefile is that specfem3d can build with xlf without workaround or error.

Note - The same problem exists in specfem2d: geodynamics/specfem2d#602 https://github.com/geodynamics/specfem2d/issues/602

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/geodynamics/specfem3d/issues/818, or mute the thread https://github.com/notifications/unsubscribe-auth/AFjDKVEvvGmF8q4iH2LWt1n1J0Mxneiiks5qdx3_gaJpZM4Je-80.

Dimitri Komatitsch CNRS Research Director (DR CNRS), Laboratory of Mechanics and Acoustics, UPR 7051, Marseille, France http://komatitsch.free.fr

dmiller423 commented 7 years ago

Pending pull request - https://github.com/dmiller423/specfem3d

komatits commented 7 years ago

Fixed by @dmiller423 ; I will make the same changes in the 2D and 3D_GLOBE version.

jlost commented 7 years ago

I'm now getting the following error:

"src/specfem3D/prepare_timerun.F90", line 697.14: 1514-051 (S) Upper bound is less than lower bound.  Please specify the -qzerosize option or use the xlf90 command to compile programs with zero-sized arrays.

Compiling with xlf90 instead of xlf does work around this, but it seems like it would be a net win for code quality to fix the offending line(s?) instead. Would that be a complicated change?

edelsohn commented 7 years ago

The .F90 file extension implies Fortran 90 semantics. Why shouldn't the source code require a compiler invoked in Fortran 90 mode?

dmiller423 commented 7 years ago

Yes it does require building with xlf90.

jlost commented 7 years ago

Good point. The fix looks good to me.

komatits commented 7 years ago

Hi all,

Yes, zero-size arrays are OK in Fortran 90 and above thus the compiler should accept that; thus yes please use xlf90

and I will add -qzerosize to flags.guess just in case

Dimitri.

On 01/23/2017 07:34 PM, James wrote:

I'm now getting the following error:

|"src/specfem3D/prepare_timerun.F90", line 697.14: 1514-051 (S) Upper bound is less than lower bound. Please specify the -qzerosize option or use the xlf90 command to compile programs with zero-sized arrays. |

Compiling with xlf90 instead of xlf does work around this, but it seems like it would be a net win for code quality to fix the offending line(s?) instead. Would that be a complicated change?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/geodynamics/specfem3d/issues/818#issuecomment-274576124, or mute the thread https://github.com/notifications/unsubscribe-auth/AFjDKUb-JWBwyh1tFgZSMOLORrwjdr8iks5rVPK9gaJpZM4Je-80.

-- Dimitri Komatitsch, CNRS Research Director (DR CNRS) Laboratory of Mechanics and Acoustics, Marseille, France http://komatitsch.free.fr

komatits commented 7 years ago

Hi all,

Done! (in 2D, 3D and 3D_GLOBE)

Dimitri.

On 01/23/2017 10:41 PM, Dimitri Komatitsch wrote:

Hi all,

Yes, zero-size arrays are OK in Fortran 90 and above thus the compiler should accept that; thus yes please use xlf90

and I will add -qzerosize to flags.guess just in case

Dimitri.

On 01/23/2017 07:34 PM, James wrote:

I'm now getting the following error:

|"src/specfem3D/prepare_timerun.F90", line 697.14: 1514-051 (S) Upper bound is less than lower bound. Please specify the -qzerosize option or use the xlf90 command to compile programs with zero-sized arrays. |

Compiling with xlf90 instead of xlf does work around this, but it seems like it would be a net win for code quality to fix the offending line(s?) instead. Would that be a complicated change?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/geodynamics/specfem3d/issues/818#issuecomment-274576124,

or mute the thread https://github.com/notifications/unsubscribe-auth/AFjDKUb-JWBwyh1tFgZSMOLORrwjdr8iks5rVPK9gaJpZM4Je-80.

-- Dimitri Komatitsch, CNRS Research Director (DR CNRS) Laboratory of Mechanics and Acoustics, Marseille, France http://komatitsch.free.fr

komatits commented 7 years ago

Hi all,

I have to revert the change because using "\0" as an empty string works for IBM xlf but it turns out that it breaks other compilers, and all other compilers accept "" for empty strings. I thus need to revert to that, and on IBM xlf since the error message is:

1515-009 (E) Null literal string is not permitted. A single blank is assumed

I add -qsuppress=1515-009 to flags.guess to fix the empty string problem.

(if anyone using xlf knows a better fix, like a -qallowemptystrings or something like that, please let us know)

Thanks, Dimitri.

komatits commented 7 years ago

Done!

dmiller423 commented 7 years ago

I tested vs gcc, it built fine.
Is it affecting build or program behaviour?

The way the code used the empty string I took it to want to be used as a null. However there are multiple ways fortran can specify string or character data, and if what is required IS a single blank, then it could be used instead of the \0 or x0. The only problem i could forsee was the possibility of existing files using something different, and a newly built binary attempting to use one an old binary wrote ( IF gcc was using a single space for an empty string/char )

So please let me know what the real issue is, since I modified the code I will be happy to see if I can help.

komatits commented 7 years ago

Thanks!

The goal is to have empty strings; when using "" or '' which is quite classical in Fortran we get this with IBM xlf:

1515-009 (E) Null literal string is not permitted. A single blank is assumed.

(thus let us try -qsuppress=1515-009 )

No problem regarding old files, people rerun the mesher to create their databases thus we are all set.

I think the compiler that complained about the switch to '\0' was Intel ifort (Bence, do you confirm?)

thanks Dimitri

On 01/27/2017 09:09 PM, David Miller wrote:

I tested vs gcc, it built fine. Is it affecting build or program behaviour?

The way the code used the empty string I took it to want to be used as a null. However there are multiple ways fortran can specify string or character data, and if what is required IS a single blank, then it could be used instead of the \0 or x0. The only problem i could forsee was the possibility of existing files using something different, and a newly built binary attempting to use one an old binary wrote ( IF gcc was using a single space for an empty string/char )

So please let me know what the real issue is, since I modified the code I will be happy to see if I can help.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/geodynamics/specfem3d/issues/818#issuecomment-275762129, or mute the thread https://github.com/notifications/unsubscribe-auth/AFjDKUMJlFIiATCHAj9OFPlvIlxBjRJZks5rWk8KgaJpZM4Je-80.

-- Dimitri Komatitsch, CNRS Research Director (DR CNRS) Laboratory of Mechanics and Acoustics, Marseille, France http://komatitsch.free.fr

komatits commented 7 years ago

I have checked more carefully, Intel ifort did not complain but then put a \0 in the output file instead of an empty string, which made ParaView crash when opening that data file.

Dimitri.

On 01/27/2017 09:19 PM, Dimitri Komatitsch wrote:

Thanks!

The goal is to have empty strings; when using "" or '' which is quite classical in Fortran we get this with IBM xlf:

1515-009 (E) Null literal string is not permitted. A single blank is assumed.

(thus let us try -qsuppress=1515-009 )

No problem regarding old files, people rerun the mesher to create their databases thus we are all set.

I think the compiler that complained about the switch to '\0' was Intel ifort (Bence, do you confirm?)

thanks Dimitri

On 01/27/2017 09:09 PM, David Miller wrote:

I tested vs gcc, it built fine. Is it affecting build or program behaviour?

The way the code used the empty string I took it to want to be used as a null. However there are multiple ways fortran can specify string or character data, and if what is required IS a single blank, then it could be used instead of the \0 or x0. The only problem i could forsee was the possibility of existing files using something different, and a newly built binary attempting to use one an old binary wrote ( IF gcc was using a single space for an empty string/char )

So please let me know what the real issue is, since I modified the code I will be happy to see if I can help.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/geodynamics/specfem3d/issues/818#issuecomment-275762129,

or mute the thread https://github.com/notifications/unsubscribe-auth/AFjDKUMJlFIiATCHAj9OFPlvIlxBjRJZks5rWk8KgaJpZM4Je-80.

-- Dimitri Komatitsch, CNRS Research Director (DR CNRS) Laboratory of Mechanics and Acoustics, Marseille, France http://komatitsch.free.fr

dmiller423 commented 7 years ago

So it is an issue of writing to the output file, it just requires a check of said output file to see what is written by using an empty string in gcc. IE: check with a hex editor, I would imagine an empty string would be of zero length = no write. From what I read however, it would output a blank character (which is still somewhat vague). If we know what the character is, we can fix it with the same method across all compilers w.o using a non standard extension. (Which there is documentation that saying empty string literals are not standard).

Either way, as a maintainer it is up to you.

komatits commented 7 years ago

Hi,

Yes, it is with the Intel compiler.

Thanks, Bence

On 01/27/2017 09:21 PM, Dimitri Komatitsch wrote:

I have checked more carefully, Intel ifort did not complain but then put a \0 in the output file instead of an empty string, which made ParaView crash when opening that data file.

Dimitri.

On 01/27/2017 09:19 PM, Dimitri Komatitsch wrote:

Thanks!

The goal is to have empty strings; when using "" or '' which is quite classical in Fortran we get this with IBM xlf:

1515-009 (E) Null literal string is not permitted. A single blank is assumed.

(thus let us try -qsuppress=1515-009 )

No problem regarding old files, people rerun the mesher to create their databases thus we are all set.

I think the compiler that complained about the switch to '\0' was Intel ifort (Bence, do you confirm?)

thanks Dimitri

On 01/27/2017 09:09 PM, David Miller wrote:

I tested vs gcc, it built fine. Is it affecting build or program behaviour?

The way the code used the empty string I took it to want to be used as a null. However there are multiple ways fortran can specify string or character data, and if what is required IS a single blank, then it could be used instead of the \0 or x0. The only problem i could forsee was the possibility of existing files using something different, and a newly built binary attempting to use one an old binary wrote ( IF gcc was using a single space for an empty string/char )

So please let me know what the real issue is, since I modified the code I will be happy to see if I can help.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/geodynamics/specfem3d/issues/818#issuecomment-275762129,

or mute the thread https://github.com/notifications/unsubscribe-auth/AFjDKUMJlFIiATCHAj9OFPlvIlxBjRJZks5rWk8KgaJpZM4Je-80.