search

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

commit e647167caf930a2666af81a9b2d310a084894774 have different results of forward simulation from previous version #1163

Closed XStargate closed 6 years ago

XStargate commented 7 years ago

I tested the injective SEM-FK method recently and found the commit e647167caf930a2666af81a9b2d310a084894774 had the different results of forward simulation from the version before the commit. Here are some comparisons:

The velocity waveforms of the radial component: New version: image Old version: image

The displacement waveforms of the vertical component: New version: image Old version: image

And here is my FKMODEL:

# 
#  input file for embedded FK modeiling 
#
#  for each layer we give : 
#  LAYER ilayer rho vp vs ztop
#  the last layer is the homogeneous half space 
#
#
# model description  ---------------------
NLAYER             2
LAYER 1 2000. 5000. 3500.     0.
LAYER 2 3600. 7000. 4500. -30000.
#LAYER 3 3423. 8060. 4530. -35000.
#LAYER 4 3423. 8289. 4517. -120000.
#----------------------------------------
# incident wave p or sv 
INCIDENT_WAVE      p
#----------------------------------------
# anlges of incomming wave
AZIMUTH                90.
TAKE_OFF               20.
#----------------------------------------
FREQUENCY_MAX      2.5
#----------------------------------------
TIME_WINDOW       64.
#----------------------------------------
# optionnal 
#ORIGIN_WAVEFRONT  1. 2. 3.
#ORIGIN_TIME 0.

I read the source code but not figured out which part that really changes the simulation. And I feel the old version before the commit seems to be right. I will appreciate it if someone can help with it.

komatits commented 7 years ago

Hi,

Thanks for your bug report. Let me cc Vadim Monteiller, since apparently the difference comes from a commit he made recently. I will check that with him tomorrow (we work in the same lab).

Thanks, Dimitri.

On 11/06/2017 09:11 PM, Xin Song wrote:

I tested the injective SEM-FK method recently and found the commit e647167 https://github.com/geodynamics/specfem3d/commit/e647167caf930a2666af81a9b2d310a084894774 had the different results of forward simulation from the version before the commit. Here are some comparisons:

The velocity waveforms of the radial component: New version: image https://user-images.githubusercontent.com/11036260/32461274-4a76bc32-c303-11e7-8aed-15c13bb7a0ae.png Old version: image https://user-images.githubusercontent.com/11036260/32461358-869107a4-c303-11e7-830d-f24ab4adfaae.png

The displacement waveforms of the vertical component: New version: image https://user-images.githubusercontent.com/11036260/32461449-d500d112-c303-11e7-97dc-b32f2e174a64.png Old version: image https://user-images.githubusercontent.com/11036260/32461402-b758f892-c303-11e7-8dd9-be5c93b22f8e.png

And here is my FKMODEL:

|# # input file for embedded FK modeiling # # for each layer we give : # LAYER ilayer rho vp vs ztop # the last layer is the homogeneous half space # # # model description --------------------- NLAYER 2 LAYER 1

        1. LAYER 2 3600. 7000. 4500. -30000. #LAYER 3 3423.
    1. -35000. #LAYER 4 3423. 8289. 4517. -120000.

      ---------------------------------------- # incident wave p or sv

      INCIDENT_WAVE p #---------------------------------------- # anlges of incomming wave AZIMUTH 90. TAKE_OFF 20.

      ---------------------------------------- FREQUENCY_MAX 2.5

      ---------------------------------------- TIME_WINDOW 64.

      ---------------------------------------- # optionnal #ORIGIN_WAVEFRONT

      1. ORIGIN_TIME 0. |

I read the source code but not figured out which part that really changes the simulation. And I feel the old version before the commit seems to be right. I will appreciate it if someone can help with it.

— 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/1163, or mute the thread https://github.com/notifications/unsubscribe-auth/AFjDKUj_sQ9cIDG5XUIJOSuBBEe_g0mEks5sz2f1gaJpZM4QTzpG.

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

vmont commented 6 years ago

Hi, on my side I didn't find the bug. Can you provide me all the input file in order to test your example? Thanks, Vadim

EtienneBachmann commented 6 years ago

Hi Vadim,

If you look at the history of the mentionned commit, you can see that you probably removed parts of the code. I saw for instance at the beginning of iterate_time that initialization of seismo_current and other variables have been removed.

Best regards,

Etienne

De : Vadim Monteiller notifications@github.com Envoyé : mardi 7 novembre 2017 19:37 À : geodynamics/specfem3d Cc : Subscribed Objet : Re: [geodynamics/specfem3d] commit e647167caf930a2666af81a9b2d310a084894774 have different results of forward simulation from previous version (#1163)

Hi, on my side I didn't find the bug. Can you provide me all the input file in order to test your example? Thanks, Vadim

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/geodynamics/specfem3d/issues/1163#issuecomment-342579456, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AIN3fvdnMZZl2z1xMEx822WN-c8eKe2yks5s0KNSgaJpZM4QTzpG.

komatits commented 6 years ago

Hi Etienne,

Thanks! Very useful.

Vadim, the diff (red and green) is at https://github.com/geodynamics/specfem3d/commit/e647167caf930a2666af81a9b2d310a084894774

Etienne is right, the red part in the attached image should still be there for instance. Probably a cut and paste that went wrong.

This is not detected by BuildBot nightly builds because FK-SEM coupling is not in the list of builbot tests / EXAMPLES I think. (clearly, it should)

thanks Dimitri.

On 11/07/2017 08:00 PM, EtienneBachmann wrote:

Hi Vadim,

If you look at the history of the mentionned commit, you can see that you probably removed parts of the code. I saw for instance at the beginning of iterate_time that initialization of seismo_current and other variables have been removed.

Best regards,

Etienne

De : Vadim Monteiller notifications@github.com Envoyé : mardi 7 novembre 2017 19:37 À : geodynamics/specfem3d Cc : Subscribed Objet : Re: [geodynamics/specfem3d] commit e647167caf930a2666af81a9b2d310a084894774 have different results of forward simulation from previous version (#1163)

Hi, on my side I didn't find the bug. Can you provide me all the input file in order to test your example? Thanks, Vadim

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/geodynamics/specfem3d/issues/1163#issuecomment-342579456, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AIN3fvdnMZZl2z1xMEx822WN-c8eKe2yks5s0KNSgaJpZM4QTzpG.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/geodynamics/specfem3d/issues/1163#issuecomment-342586288, or mute the thread https://github.com/notifications/unsubscribe-auth/AFjDKaUqohLiRJHoekHMdd1TZXN26V31ks5s0KjIgaJpZM4QTzpG.

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

XStargate commented 6 years ago

Hi Vadim,

Here are the input files of my examples: The input files of old version: DATA_old.tar.gz The input files of new version: DATA_new.tar.gz

I hope it is helpful for you to test it.

Thanks, Xin

komatits commented 6 years ago

Hi all,

Vadim has checked, there is no bug (the red part in the picture I attached was just moved elsewhere). It is just that there is an inconsistency in the input file used for the tests; Vadim will send all the details tomorrow.

Best, Dimitri.

On 11/07/2017 08:05 PM, Dimitri Komatitsch wrote:

Hi Etienne,

Thanks! Very useful.

Vadim, the diff (red and green) is at https://github.com/geodynamics/specfem3d/commit/e647167caf930a2666af81a9b2d310a084894774

Etienne is right, the red part in the attached image should still be there for instance. Probably a cut and paste that went wrong.

This is not detected by BuildBot nightly builds because FK-SEM coupling is not in the list of builbot tests / EXAMPLES I think. (clearly, it should)

thanks Dimitri.

On 11/07/2017 08:00 PM, EtienneBachmann wrote:

Hi Vadim,

If you look at the history of the mentionned commit, you can see that you probably removed parts of the code. I saw for instance at the beginning of iterate_time that initialization of seismo_current and other variables have been removed.

Best regards,

Etienne

De : Vadim Monteiller notifications@github.com Envoyé : mardi 7 novembre 2017 19:37 À : geodynamics/specfem3d Cc : Subscribed Objet : Re: [geodynamics/specfem3d] commit e647167caf930a2666af81a9b2d310a084894774 have different results of forward simulation from previous version (#1163)

Hi, on my side I didn't find the bug. Can you provide me all the input file in order to test your example? Thanks, Vadim

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/geodynamics/specfem3d/issues/1163#issuecomment-342579456, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AIN3fvdnMZZl2z1xMEx822WN-c8eKe2yks5s0KNSgaJpZM4QTzpG.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/geodynamics/specfem3d/issues/1163#issuecomment-342586288, or mute the thread https://github.com/notifications/unsubscribe-auth/AFjDKaUqohLiRJHoekHMdd1TZXN26V31ks5s0KjIgaJpZM4QTzpG.

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

vmont commented 6 years ago

Hi Xin, I just ran your example with the current version of specfem and I obtained the following sismo for Z component. sismoz I found that your Par_file for new test used a different model that you used in old test. In your new test you use a tomographic model that is not seems to be flat layer as the previous you used in old test. I just used the layer model and I obtained the same results with new code and old code.

Vadim

XStargate commented 6 years ago

Hi Vadim,

I just tested it with default model and got the same results. I am sorry that I missed that inconsistency and thank you for your kind help.

Thx, Xin

komatits commented 6 years ago

Hi Xin,

No problem at all, you were right to contact the mailing list to check, it is much safer to check than to potentially leave a bug.

Thanks again, Best regards, Dimitri.

On 11/08/2017 01:09 AM, Xin Song wrote:

Hi Vadim,

I just tested it with default model and got the same results. I am sorry that I missed that inconsistency and thank you for your kind help.

Thx, Xin

— 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/1163#issuecomment-342666410, or mute the thread https://github.com/notifications/unsubscribe-auth/AFjDKeWGvWpvCcl8e0rccBDAkVDzh5Vkks5s0PE2gaJpZM4QTzpG.

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