Forcing high Q around source node

[deanrockit]

When using the new source input, the result is the same no matter FORCE_HIGH_Q is 0 or 1, whereas it will be executed and also take effect with old source input when FORCE_HIGH_Q is enabled. With new source input and FORCE_HIGH_Q enabled, this part of the code was indeed executed since I saw "force high q around source nodes" in the output file, but it didn't take effect.

I guess the reason is that AWP doesn't know where the source is at this point when we use the new source input, so that no any value is assigned to the "tpsrc" array. Therefore the lines below aren't changing anything in the modeling domain (even though there's a bug, but doesn't matter).                  for (xi=idx-2; xi<idx+3;xi++){             for (yi=idy-2; yi<idy+3;yi++){                for (zi=idz-2; zi<idz+3;zi++){                   dox=doy=doz=0;                   if ((xi>=0) && (xi < (nxt[0] + ngsl2 +1))) dox = 1;                   if ((yi>=0) && (yi < (nyt[0] + ngsl2 +1))) doy = 1;                   //FIXME: Bug here? shouldn't it be zi < (nzt[0] + ...)                   if ((zi>=0) && (yi < (nzt[0] + ngsl2 +1))) doz = 1;                   if ((dox && doy) && doz ) {                      qp[p][xi][yi][zi]=7.88313861E-04;  //Q of 10,000 before inimesh                      qs[p][xi][yi][zi]=7.88313861E-04;                      //qp[p][xi][yi][zi]=0.;  //Q of 10,000 before inimesh                      //qs[p][xi][yi][zi]=0.;                   }                }              }

So, yea, source/receiver implementation is okay, but there are still some differences between the two versions when topography is disabled.

[ooreilly]

@deanrockit Should the change of Q values take place? If so, we need to make sure that the change is correct and also works for both old and new sources.

Otherwise please submit a fix that removes the macro and block of code.

[deanrockit]

Yes it does, but it will be done to somewhere else (very likely in the ghost layer) but not around the source since the source location is not yet defined at this point when the new source input is used.

In order to make it work for both old and new inputs, we will need to change how the sources are initialized in case of the new source input so that AWP knows where the sources are before changing the Q values. Not sure if you would like to go this route because we don't intend to use this block anyway. Maybe removing it from the code is a better idea? Or keeping it as a conditional macro and disable it by default?

[ooreilly]

@deanrockit I'm guessing that in your explanation

Yes it does, but it will be done to somewhere else (very likely in the ghost layer) but not around the source since the source location is not yet defined at this point when the new source input is used.

you are pointing out that the new source implementation modifies the Q values when FORCE_HIGH_Q 1, but the location at which they are being modified is not around the source nodes.

If you do not intend using the feature either now or in the future, then let's delete it. Can you provide the fix and open a pull request? Thanks.

[hzfmer]

I don't think we will need this feature anymore, except when we do a two-step simulation, which means running a dynamic simulation first and then use the resulting stress/velocity field as an input to a kinematic simulation in a larger domain. I would suggest keeping some notes on how this part works and why it is deleted if we remove it.

[ooreilly]

@hzfmer we will reference this issue when merging the PR. Since you may want to use the feature in certain cases, I propose a different strategy:

1. Remove the feature for now
2. Create a new issue that explains what to implement
3. Provide the implementation. Here is what I find important:
   • Instead of the macro solution, add a new command line argument option
   • Remove the hard-coded Q values
   • Disable the feature by default
   • Document what the feature does
4. Provide a test that demonstrates the correct behavior of the code when the feature is running (we can discuss ideas on how to proceed with that). As far as I can tell from reading the code, the feature does not work as intended at the moment.

[hzfmer]

@ooreilly I agree. Let's remove the feature for now. We should focus on making sure source locations are correct now. The FORCE_HIGH_Q issue could be solved later.

[deanrockit]

4. As far as I can tell from reading the code, the feature does not work as intended at the moment.

No it doesn't. There's a bug, especially. And it's not needed for the wave propagation mode, which is the "only" option in the current version of AWP. We may document this down about how to put it back in case it's needed again. Let's just remove it from the code and move on. At least we know the source/receiver implementation is working as expected.