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
409 stars 227 forks source link

extend allowable length of filename sizes for tomography model (model_tomography.f90) #128

Closed carltape closed 10 years ago

carltape commented 10 years ago

I think I isolated a problem. I am reading in an external tomography file with MODEL = tomo set in Par_file. If I use this for nummaterial_velocity_file 2 -1 tomography elastic file1.xyz 1 then everything works. Here file1.xyz is a symbolic link to an xyz tomo file. If I then use this 2 -1 tomography elastic homogeneous_halfspace_tomo_dx04dy04dz01.xyz 1 (where the long name is pointing to the SAME tomography file) it will exist with error messages "error reading tomography file" which are coming from model_tomography.f90 So it appears that the way that the file is written in nummaterial_velocity_file matters! Will extending the string lengths solve the problem?

QuLogic commented 10 years ago

There is a miscalculation in the lengths:

character(len=256) OUTPUT_FILES,LOCAL_PATH,TOMOGRAPHY_PATH,TRAC_PATH
character(len=256):: tomo_filename
character(len=256):: filename

and

tomo_filename = TOMOGRAPHY_PATH(1:len_trim(TOMOGRAPHY_PATH))//trim(filename)

Obviously, tomo_filename needs to be 256+256=512 characters in order to fit the maximum TOMOGRAPHY_PATH+filename.

komatits commented 10 years ago

You can type this to see all of them in the source code:

grep -i -n --color=always character src/_/_90 | grep -i len

and then increase the length of strings that seem too short and commit the changes to the GIT "devel" branch.

Most of them seem to be equal to 256 or 512, which seems fine, but I see a few that are equal to 80 or to 150 and that should probably be increased.

On 23/04/2014 22:39, Elliott Sales de Andrade wrote:

There is a miscalculation in the lengths:

character(len=256) OUTPUT_FILES,LOCAL_PATH,TOMOGRAPHY_PATH,TRAC_PATH character(len=256):: tomo_filename character(len=256):: filename

and

tomo_filename = TOMOGRAPHY_PATH(1:len_trim(TOMOGRAPHY_PATH))//trim(filename)

Obviously, |tomo_filename| needs to be 256+256=512 characters in order to fit the maximum |TOMOGRAPHY_PATH|+|filename|.

— Reply to this email directly or view it on GitHub https://github.com/geodynamics/specfem3d/issues/128#issuecomment-41211707.

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

komatits commented 10 years ago

@carltape @QuLogic are you done fixing this? if so could you close the issue?

komatits commented 10 years ago

Fixed by @QuLogic

komatits commented 10 years ago

Hi Carl @carltape Did you reopen this purposely? If so, could you describe the current status of the problem?

carltape commented 10 years ago

As of the 6/1/2014 version, I think there is still a problem. If I use nummaterial_velocity_file like this

2 -1 tomography elastic cvm119sm_1000_1000_0250_basin_vsmin_0600.xyz 1 2 -2 tomography elastic cvm119sm_2000_2000_1000_crust_vsmin_0600.xyz 1

it will exit when generating databases with the messages

pacman12 % more error_message000004.txt error reading tomography file Error detected, aborting MPI... proc 4

If I then make a symbolic link to the same files and make this nummaterial_velocity_file file

2 -1 tomography elastic file1.xyz 1 2 -2 tomography elastic file2.xyz 1

then it works fine. I don't think it has to do with the fact that there are two files. But this seems to be the same problem that I identified before.

QuLogic commented 10 years ago

Hi Carl,

The change that I made does not increase any string lengths. It only introduces a constant so that it is easier to increase it in all the code. If you want to try with a longer string length, then you can change the constant MAX_STRING_LEN in constants.h.

komatits commented 10 years ago

I have just increased it to 512

carltape commented 10 years ago

I tried it with MAX_STRING_LEN=1024 before I posted, and I still encountered the error. I will try everything from scratch. There's no way that I would need strings longer than 1024 (or even 256) for file path names, so this does not seem to be solved by changing MAX_STRING_LEN.

casarotti commented 10 years ago

did you print out the value of tomo_filename at row 110 of model_tomography.f90?

open(unit=27,file=trim(tomo_filename),status='old',action='read',iostat=ier)

On Wed, Jun 4, 2014 at 11:12 PM, Carl Tape notifications@github.com wrote:

I tried it with MAX_STRING_LEN=1024 before I posted, and I still encountered the error. I will try everything from scratch. There's no way that I would need strings longer than 1024 (or even 256) for file path names, so this does not seem to be solved by changing MAX_STRING_LEN.

— Reply to this email directly or view it on GitHub https://github.com/geodynamics/specfem3d/issues/128#issuecomment-45152272 .

carltape commented 10 years ago

Okay, I added a few print lines around line 110 of model_tomography.f90.

read(undef_matprop(4,iundef),) filename if( myrank == 0 ) write(6,_) '',trim(TOMOGRAPHYPATH) if( myrank == 0 ) write(6,) '',trim(filename) tomo_filename = TOMOGRAPHY_PATH(1:len_trim(TOMOGRAPHYPATH))//trim(filename) if( myrank == 0 ) write(6,) '',trim(tomo_filename)

Here is the output

./DATA/ cvm119sm_1000_1000_0250basin ./DATA/cvm119sm_1000_1000_0250basin error reading tomography file Error detected, aborting MPI... proc 0

The nummaterial_velocity_file is this

2 -1 tomography elastic cvm119sm_1000_1000_0250_basin_vsmin_0600.xyz 1 2 -2 tomography elastic cvm119sm_2000_2000_1000_crust_vsmin_0600.xyz 1

It appears that the variable "filename" is not being properly read. (This has nothing to do with the fact that I have two files to read, I think.)

This all works fine if I replace the long names with things like file1.xyz and file2.xyz, but this is a silly workaround. I'm not that great with string read/write. Help wanted!

QuLogic commented 10 years ago

Looks like I missed one string because it was not the original 256.

QuLogic commented 10 years ago

Please try with #154.

carltape commented 10 years ago

Excellent, thanks Elliott! The problem is now gone for me. It looks like one had been set at length 30, which is pretty short. Though we might want to step down MAX_STRING_LEN from 512 (current) to 256 (original). It's hard to imagine someone wanting a default string 512 long, though probably the storage cost is irrelevant.