grdimage [ERROR]: Option -x given more than once

[weiji14]

Description of the problem

Running grdimage with the -x (cores) option fails on GMT 6.4.0 and GMT 6.5.0.dev9+0776994. It seems to think the -x option is being passed in more than once? Noticed at GenericMappingTools/pygmt#2945 (comment)

Full script that generated the error

gmt grdimage @earth_relief_01d_g -RGL -Cgeo -x2 -png map

Full error message

grdimage [ERROR]: Option -x given more than once
grdimage [ERROR]: Option -x parsing failure. Correct syntax:

   [-x[[-]<n>]] .
     Limit the number of cores used in multi-threaded algorithms [Default uses all 16 cores]. If <n> is negative then we select (16 - <n>) cores (or at least 1).
grdimage [ERROR]: Offending option -x2

Actual outcome

grdimage errors

Expected outcome

A map like this should be produced:

System information

• Operating system: Linux
• GMT version (gmt --version): 6.4.0

[PaulWessel]

Hm, I can reproduce the same. Very odd since

1.. If I build with Xcode there is no OPENMP available yet I still get that message. Traced to

#ifdef GMT_MP_ENABLED
		case 'x': error += GMT->common.x.active == false; break;
#endif

GMT_MP_ENABLED means a guru wishes to build with OPenMP if possible but that does not happen in Xcode, yet GMT_MP_ENABLED is true. _OPENMP is not set.

If I wrap the parsing under #ifdef _OPENMP then it more correctly gives the error

grdimage [ERROR]: Option -x is not a recognized common option

since it is not available without OpenMP.

However, when I build with gcc and enable openMP the same command yields what you got:

grdimage [ERROR]: Option -x given more than once
grdimage [ERROR]: Option -x parsing failure. Correct syntax:

   [-x[[-]<n>]]
     Limit the number of cores used in multi-threaded algorithms [Default uses all 10 cores]. If <n> is negative then we select (10 - <n>) cores (or at least 1).
grdimage [ERROR]: Offending option -x2

I cannot debug the MP version but even the non-MP debug in Xcode does show me that "-x2" is passed twice, but the linked options produced by Create_Options only has it once. So somewhere in GMT_Parse_Common it is passed two times to gmt_parse_common_options. Will do some more checking.

Ideally if not MP I would prefer a message like

grdimage [ERROR]: Option -x is not a recognized common option unless OpenMP is enabled

but it comes from a common macro.

[PaulWessel]

Yikes, more problems that that. I think this started to happen when we decided that if gthreads is used then we do this:

#if defined(HAVE_GLIB_GTHREAD) || defined(_OPENMP)
/* This means we should enable the -x+a|[-] common option */
#define GMT_MP_ENABLED
#endif

so I was wrong in previous post. Bt, the wast amount of modules that can use openMP, not gthreads (e.g., grdfilter and a few tools in supplements potential, so turning -x on in the pure OpenMP modules is what gives these errors. I think the solution is to have two
GMT_x_OPT, GMT_ADD_x_OPT
versions, set separately for _HAVE_MP and HAVE_GLIB_GTHREAD, e.g.

GMT_x_OPT_MP, GMT_ADD_x_OPT_MP and the same for GTHREAD. Then, the modules needing -x should use the MP option except for a few that needs to GTHREAD option. Does that make sense @joa-quim and @weiji14 ? Means we need to finish this work before we release 6.5.

[PaulWessel]

Oh, and then on system with both MP and GTHREAD, what do we do? Always select MP if both are present, i.e., if _HAVE_MP set those args else if GHTRED set the other constants, and if neither set them to blank?

[PaulWessel]

Final findings: None of the above was really relevant. The problem was strictly a grdimage bug. As you know, all modules have a THIS_MODULE_OPTIONS string which is used by GMT_Parse_Options to know what options are allowed. However, in a long past GMT version there were no -x option and in classic mode we internally allowed -x -y for relative -X -Y. This was also long ago abandoned but in grdimage it had xy in that string, plus a final x from GMT_ADD_x_OPT. So indeed, we passed -x twice for parsing.

Minor code changes:

1. In GMT_Parse_Common, the loop over non-critical options called gmt_parse_common_options on opt->option before the check if the current option was on the current list[0].
2. Comment added in gmt_synopsis.h what GMT_MP_ENABLED means.
3. The removal of xy from grdimage's THIS_MODULE_OPTIONS.
4. Three of six test/grdfilter test scripts did not properly check if -x should be used and thus now gave error. Updated based on the ones that pass (i.e., run without -x if not enabled).

PR coming once I've run tests.

[weiji14]

Thanks @PaulWessel, appreciate the fix. But... PyGMT's dev builds from about 6 hours ago now errors at https://github.com/GenericMappingTools/pygmt/actions/runs/7416568349/job/20181749581#step:15:908 (build from GMT master) with Error: grdfilter [ERROR]: Unrecognized option -x, and same for grdlandmask, grdsample, sph2grd 😅 Let me check if we build with OpenMP first (edit: yes https://github.com/GenericMappingTools/pygmt/blob/9221d0877ab754f97961d89ff1d53dd89ade6f2b/.github/workflows/ci_tests_dev.yaml#L129, so the fix might have broken other modules...)

[seisman]

Let me check if we build with OpenMP first (edit: yes https://github.com/GenericMappingTools/pygmt/blob/9221d0877ab754f97961d89ff1d53dd89ade6f2b/.github/workflows/ci_tests_dev.yaml#L129, so the fix might have broken other modules...)

Actually no. On Linux and macOS, we're using this script https://github.com/GenericMappingTools/gmt/blob/master/ci/build-gmt.sh to build GMT source codes.

[weiji14]

Let me check if we build with OpenMP first (edit: yes https://github.com/GenericMappingTools/pygmt/blob/9221d0877ab754f97961d89ff1d53dd89ade6f2b/.github/workflows/ci_tests_dev.yaml#L129, so the fix might have broken other modules...)

Actually no. On Linux and macOS, we're using this script https://github.com/GenericMappingTools/gmt/blob/master/ci/build-gmt.sh to build GMT source codes.

Oh nice, false alarm then (should have looked at the Windows build for once). We should improve that bash script after the 6.5.0 release.

[PaulWessel]

Yes, I just build both Xcode version (no OMP but Gthreads) and OPenMP + Gthreads version and if you run a a grd* module that has OpenMP but not actually linked that way you cannot use -x since it is not a vaiid option. We have one issue that Joaquim and I decided to punt on until 6.5 is out and that is the different behavior of -x for his Gthreads modules and all the OpenMP modules: For the OpenMP modules, if compiled to use OpenMP, the default behavior is to actually use all the available core. If you dont want that then you use -x to specify how many. For the 4 Gthreads modules the default is do not use Threads unless -z+a is used. We wish all parallel executables to behave the same way (Default is using all cpus and the +a modifier will be history).