simplify region codes in src/postprocess

[rmodrak]

Dear all,

Currently, in src/postprocess, there are three different conventions concerning region prefixes. Sometimes region prefixes are hardwired into code (e.g. reg_name = '_reg1_'). Other times a constants file is used (e.g. use postprocess_par,only: reg). Other times a prefix is determined based on a command line argument.

To make things simpler, I'd like move to a single convention: require users to include the region code explicitly through the command line arguments.
​
old convention

mpirun bin/xsmooth_sem 10. 10. rho_kernel INPUT_DIR OUTPUT_DIR

proposed new convention

mpirun bin/xsmooth_sem 10. 10. reg1_rho_kernel INPUT_DIR OUTPUT_DIR

This change would not affect any SPECFEM3D utilities, and only one of the 'classic' SPECFEM3D_GLOBE utilities, smooth_sem.f90, would be affected. (If Daniel wants me to modify the recently added utilities addition_sem and difference_sem utilities, I could do that as well.)

If there are any concerns, please let me know.

This is the final proposed change concerning the postprocess utilities--thanks for bearing with me.

Ryan

[komatits]

Hi Ryan,

What about suppressing the region suffix from the command line and
smoothing all the regions (i.e. reg1 reg2 reg3) automatically (if the
corresponding kernel exists on the disk, if not, ignoring that region).

Otherwise this would make the syntax differ from SPECFEM3D, and I am not
sure someone would smooth a region without smoothing the other (?).
Of course some kernels are in the crust_mantle region only, if so for
these kernels reg2 and reg3 are meaningless; (but then they can probably
be smoothed anyway even if that is useless, because smoothing reg2 is
cheap and smoothing reg3 is very cheap).

Anyway, if possible let us avoid making the syntax of GLOBE tools differ
from the rest, since we are trying to do the opposite.

Other option: by default smooth all the regions if no "reg_" argument is
present on the command line, but accept an optional "reg_" argument if
the user wants to smooth a specific region only; as long as it is
optional we do not break compatibility with SPECFEM3D syntax.

Thanks,
Dimitri.

On 03/03/2015 10:09 PM, rmodrak wrote:

Dear all,

Currently, in src/postprocess, there are three different conventions
concerning region suffixes. Sometimes region prefixes are hardwired into
code (e.g. |reg_name = 'reg1'|). Other times a constants file is used
(e.g. |use postprocess_par,only: reg|). Other times a prefix is
determined based on a command line argument.

To make things simpler, I'd like move to a single convention: require
users to include the region code explicitly through the command line
arguments.
​
old convention

|bin/xsmooth 10. 10. rho_kernel INPUT_DIR OUTPUT_DIR
|

proposed new convention

|bin/xsmooth 10. 10. reg1_rho_kernel INPUT_DIR OUTPUT_DIR
|

This change would not affect any SPECFEM3D utilities, and only one of
the 'classic' SPECFEM3D_GLOBE utilities, |smooth_sem.f90|, would be
affected. (If Daniel wants to modify the recently added utilities
|addition_sem| and |difference_sem| utilities, I could do that as well.)

If there are any concerns, please let me know.

This is the final proposed change concerning the postprocess
utilities--thanks for bearing with me.

Ryan

—
Reply to this email directly or view it on GitHub
#338.

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

[rmodrak]

Hi Dimitri,

That is an excellent point. Thanks very much for the feedback.

There is a sense in which the proposed change improves consistency between packages by removing a hardwired variable from SPECFEM3D_GLOBE that is not present in SPECFEM3D.

On the other hand though, I do agree very much with your view.

If I could, let me think more about this from the standpoint of trying to interface with xsmooth_sem from an external script. Also, let me follow up with Daniel about this, since I brought it up with him earlier.

I'll report back later this week, and in the meantime I'll hold off on any changes.

Thanks,
Ryan

[komatits]

Hi Ryan,

Perfect!

Using an optional argument could be an option;
I am not so sure about using an external script (doing that would force
users to change the way they use these codes, i.e. currently calling
them directly from the command line; if possible, let us not change that).

Thanks,
Dimitri.

On 03/04/2015 06:14 AM, rmodrak wrote:

Hi Dimitri,

That is an excellent point. Thanks very much for the feedback.

There is a sense in which the proposed change improves consistency
between packages by removing a hardwired variable from SPECFEM3D_GLOBE
that is not present in SPECFEM3D.

On the other hand though, I do agree very much with your view.

If I could, let me think more about this from the standpoint of trying
to interface with xsmooth_sem from an external script. Also, let me
follow up with Daniel about this, since I brought it up with him earlier.

I'll report back later this week, and in the meantime I'll hold off on
any changes.

Thanks,
Ryan

—
Reply to this email directly or view it on GitHub
#338 (comment).

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

[rmodrak]

Hi Dimitri,

Sorry, all I meant was, anyone using these utilities must call them from an external script, typically a bash script. This is the approach established by Carl and Daniel.

Of course, we need to try to avoid breaking existing command line interfaces. But if we can extend command line interfaces in a way that provides more flexibility or that anticipates future user needs, there are real benefits for everyone.

Ryan

[rmodrak]

Hi Dimitri,

If it's alright, why don't we just leave everything as it is and go ahead and close this issue.

Thanks,
Ryan

[komatits]

Hi Ryan,

Ah, OK, then we are all set. So let us implement your initial idea
(getting rid of hardwired region codes and adding that to the kernel
name in the command line arguments). That will make the current routines
much cleaner.

Thanks,
Dimitri.

On 03/04/2015 02:39 PM, rmodrak wrote:

Hi Dimitri,

Sorry, all I meant was, anyone using these utilities must call them from
an external script, typically a bash script. This is the approach
established by Carl and Daniel.

Of course, we need to try to avoid breaking existing command line
interfaces. But if we can extend command line interfaces in a way that
provides more flexibility or that anticipates future user needs, there
are real benefits for everyone.

Ryan

—
Reply to this email directly or view it on GitHub
#338 (comment).

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

[komatits]

Hi Ryan,

I think your idea of cleaning that part of the code was very good, so
let us implement your initial idea (getting rid of hardwired region
codes and adding that to the kernel name in the command line arguments).
That will make the current routines much cleaner.

Thanks,
Dimitri.

On 03/04/2015 07:22 PM, rmodrak wrote:

Hi Dimitri,

If it's alright, why don't we just leave the everything as it is and go
ahead and close this issue.

Thanks,
Ryan

—
Reply to this email directly or view it on GitHub
#338 (comment).

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

[rmodrak]

Hi Dimitri,

I can go ahead and simplify the routines.

What about if we keep the region codes, but instead of hardwiring them in every single routine, or having to include them explicitly through a command line argument, we do something like use postprocess_par, only: reg_code?

From your previous comments, I think you would like this solution, and it would work well for me and Daniel too.

Thanks,
Ryan

[komatits]

Hi Ryan,

You are right, very good idea.

Thanks,
Dimitri.

On 03/05/2015 02:46 AM, rmodrak wrote:

Hi Dimitri,

I can go ahead and simplify the routines.

What about if we keep the region codes, but instead of hardwiring them
in every single routine, or having to include them explicitly through a
command line argument, we do something like |use postprocess_par, only:
reg_code|?

From your previous comments, I think you would like this solution, and
it would work well for me and Daniel too.

Thanks,
Ryan

—
Reply to this email directly or view it on GitHub
#338 (comment).

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

[rmodrak]

UPDATE:

I've gone ahead and addressed this issue in a new pull request #342 .

If it's alright, I'll go ahead in close this issue.