GenericMappingTools / gmt

The Generic Mapping Tools
https://www.generic-mapping-tools.org
Other
843 stars 352 forks source link

GMT_HISTORY=false doesn't work as expected #3643

Closed seisman closed 4 years ago

seisman commented 4 years ago

As I understand it, setting GMT_HISTORY to false means the -J and -R settings won't be written to the gmt.history file. Is it correct?

Here is an example:

gmt psbasemap -R0/10/0/10 -JX10c -Baf -K > map.ps
cat gmt.history
gmt psxy -JX1c -R0/1/0/1 -T -K -O --GMT_HISTORY=false >> map.ps
cat gmt.history
echo 5 5 | gmt psxy -Sc0.2c -R -J -O >> map.ps
cat gmt.history
rm gmt.history

Here are the content of the gmt.history file:

# GMT 6 Session common arguments shelf
BEGIN GMT 6.2.0
B   af
J   X
JX  X10c
R   0/10/0/10
@L  1
END
# GMT 6 Session common arguments shelf
BEGIN GMT 6.2.0
B   af
J   X
JX  X1c
R   0/1/0/1
@L  2
END
# GMT 6 Session common arguments shelf
BEGIN GMT 6.2.0
B   af
J   X
JX  X1c
R   0/1/0/1
@L  3
END

It seems the second command still writes its -J and -R to gmt.history, even though GMT_HISTORY is false.

seisman commented 4 years ago

Ping @PaulWessel to check whether this is a bug or a misunderstanding.

PaulWessel commented 4 years ago

Sorry the slow feedback. This was something that Florian implemented. Looking at the code that parses GMT_HISTORY, I see

        case GMTCASE_GMT_HISTORY:
            if      (strspn (lower_value, "1t"))
                GMT->current.setting.history = (GMT_HISTORY_READ | GMT_HISTORY_WRITE);
            else if (strchr (lower_value, 'r'))
                GMT->current.setting.history = GMT_HISTORY_READ;
            else if (strspn (lower_value, "0f"))
                GMT->current.setting.history = GMT_HISTORY_OFF;
            else
                error = true;

It is a bit odd and cryptic. I am guessing the strspn looks for a "1" or the t in "true", and sets the default: read and write history. And I guess anything starting with f will turn it off, and any word starting with r, such as rotten or read or readonly, will turn on reading only. I think we want to strengthen this parsing to be less tolerant. Then in put_history we have this check

    if (!(GMT->current.setting.history & GMT_HISTORY_WRITE)) {
        if (GMT->current.setting.run_mode == GMT_MODERN && GMT->current.setting.history == GMT_HISTORY_OFF)
            GMT->current.setting.history = GMT->current.setting.history_orig;
        return (GMT_NOERROR); /* gmt.history mechanism has been disabled */
    }

which seems to prevent writing. And in get_history we have

    if (!(GMT->current.setting.history & GMT_HISTORY_READ))
        return (GMT_NOERROR); /* gmt.history mechanism has been disabled */

which would also seem to skip the reading. So your examples above is classic but PyGMT does modern, no?

seisman commented 4 years ago

Yes, PyGMT uses modern mode. I also ran the following commands one by one, and check the gmt.history file in the session directory:

gmt begin
gmt basemap -R0/10/0/10 -JX10c -Baf
gmt plot -JX1c -R0/1/0/1 -T --GMT_HISTORY=false

After running the gmt plot command, I still see

# GMT 6 Session common arguments shelf
BEGIN GMT 6.2.0
J   X
JX  X1c
R   0/1/0/1
@L  2
END
PaulWessel commented 4 years ago

OK, I will need to add kerosen and fire up the old Xcode debugger...

PaulWessel commented 4 years ago

Suspicion is that gmt.history is read during Create_Session before any GMT_HISTORY is processed. Confirmed by running gmt set GMT_HISTORY false instead of via command line. I think we will need to scan for --GMT_HISTORY before get_history is called, otherwise it is always read before --PAR=value are parsed. As for writing, that happens at the end. Note taht if you really turn it to false then there is no reading and

echo 5 5 | gmt psxy -Sc0.2c -R -J -O >> map.ps psxy [ERROR]: Found no history for option -R pwessel@macnut:~->

PaulWessel commented 4 years ago

From what I can tell, all the internal cases where we call GMT_Call_Module with --GMT_HISTORY=false we provide the needed -Rargs -Jargs but do not want them to be added. So while in all those case we read teh history, it gets internally replaced by the specific -R -J settings, but these are then now written out. So, If we intervene earlier and turn reading the history off in this case it should not have any negative effects, and if it does the perhaps we should pass GMT_HISTORY=readonly since that will still prevent the writing.

PaulWessel commented 4 years ago

Furthermore, GMT_HISTORY=false is properly parsed by the module and GMT->current.setting.history = 0. But when _gmtend is called we clobber that copy of the GMT struct and replace it with what the calling program had, hence GMT->current.setting.history = 3 again, and we write the history.

Discussion:

There is a git difference between running gmt set GMT_HISTORY false before running another module and passing --GMT_HISTORY=false on the module command line. It is the same for all such parameters: In the first case, gmt.conf is updated and then the next module reads that and replaces its default history = 3 setting with what is in gmt.conf. This sets the history setting for the calling program gmt. When it calls the module then the module inherits that setting. More importantly, when the module returns the original setting via gmt.conf is restored (0) and gmt_end is told not to wrote the history. With --GMT_HISTORY=false the parsing takes place when the module is called. INternally, history becomes 0, but it is not really consulted until _gmtend is called. By that time the module is already finished and the original default value prevails (3).

Some GMT defaults set inside a module do survive up the chain of command. We set these explicitly in the _gmtend function after dealing with the old and new GMT structure. We can impose the rule that GMT_HISTORY set in the module will percolate up but not sure if we want that. However, there is no mechanism to prevent a module from reading gmt.history via --GMT_HISTORY=false since _GMT_CreateSession does not consider the module options and hence it can never have the information needed. However, it is very unusual to want a module to not read the history, and it is easily implemented by a gmt set command. The same is true of writing the history of course.

Conclusion:

I think we should just implement these relative cosmetic changes:

  1. TIghten the parsing of the argument to GMT_HISTORY. Looking at a leading f is a bit vague.
  2. In all the internal calls where --GMT_HISTORY=false are passed we should replace that with --GMT_HISTORY=readonly. That is not only more accurate but also is adequate for what is attempted by modules calling a module
  3. For other external uses, like PyGMT, it seems you will need to manage that history via gmt set. Alternatively, if it proves more convenient, we could add another bit flag to the GMT_Create_Session mode argument that allows you to set the history flag for that session (read, read|write, or neither). I think this is a good solution since it does not affect gmt at all.
  4. The gmt.conf discussion of GMT_HISTORY should include some of these elements.

Let me know @seisman what makes more sense.

PaulWessel commented 4 years ago

My understanding of PyGMT is that each call is its own session (like the command line gmt), so do you rely on the history in any way to pass -R -J between calls, and when do you want to prevent it?

PaulWessel commented 4 years ago

Finally, whether or not a mode flag is useful for PyGMT it would seem it is a good mode option anyway to ensure that a single session does not at all use the short-hand history mechanism.

seisman commented 4 years ago

do you rely on the history in any way to pass -R -J between calls

I think yes.

and when do you want to prevent it?

The only exception is, PyGMT provides a function shift_origin() to shift the plot origin. It internally calls gmt plot -T -Xxshift -Yyshift, so it relies on the -R -J history to work. However, it doesn't work/crashes if it's the first "plotting" command, because there is no -R -J history available. Then I thought I can call gmt plot -R0/1/0/1 -Jx1c -T -Xxshift -Yyshit --GMT_HISTORY=false which works no matter it's the first command or not.

PaulWessel commented 4 years ago

Hm. So while plot certainly expect -R -J, it does not need that to do -X -Y since those are always i plot units. And with -T there is no data plotted, and the only thing that needs -R -J would be -B. So I could probably allow plot to not require -R -J if -T is used and there is no -B.

seisman commented 4 years ago

So I could probably allow plot to not require -R -J if -T is used and there is no -B.

Yes, it's a better solution for shifting plot origins.

PaulWessel commented 4 years ago

We removed -T from plot (and hence) psxy documentation but the option has no restrictions on run-mode. I think we did because we could not see a use for it in modern mode but here you are. I am adding the option back to the psxy man page for sure; the question becomes should we do it for modern mode as well. Maybe @joa-quim also uses this trick to change origins in Julia?

seisman commented 4 years ago

we did because we could not see a use for it in modern mode but here you are

subplot mode doesn't allow complex and irregular layout. That's why plot -T is still necessary.

PaulWessel commented 4 years ago

OK, so I should just add -T back to the plotcommon.rst file then.

joa-quim commented 4 years ago

I use -T to close the PS files. And that was the responsible for that white corner in #3539