Skip to content

Let gmt.history and gmt.conf be hierarchical and separate for panels and figures #3523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Jun 26, 2020

Conversation

PaulWessel
Copy link
Member

@PaulWessel PaulWessel commented Jun 23, 2020

We do not want an automatic region determined for one panel to apply to another panel.
This addresses #3520 but still needs some more work. It also extends this idea to gmt.conf which now is also hierarchical. Hence, current cpt, settings and history are now all hierachical. This leaves us with ~12 failures due to syntax that is now incorrect.

@PaulWessel PaulWessel changed the title WIP Let gmt.history be hierarchical and separate for panels and fitures WIP Let gmt.history and gmt.conf be hierarchical and separate for panels and figures Jun 23, 2020
@PaulWessel PaulWessel requested review from joa-quim and seisman June 23, 2020 07:23
@PaulWessel
Copy link
Member Author

PaulWessel commented Jun 23, 2020

Still to do:

  1. Update the 11 failing scripts to follow the new hierarchical system
  2. Update the documentation to extend the hierarchical explanation to settings and history as well as cpt.

I suggest @joa-quim and @seisman try this branch, I have tested it on things like this and the levels work. If you run one command at the time and monitor what is happening in the .gmt/sessions/gmt_session.* directory you will see many gmt.conf and gmt.history files now come and go at the right times. The example below sets a figure-level title font (not used), then a subplot-level font (used for all panels), except overridden for one panel. One panel also overrides the interpolant to be cubic but it is back to being Akima at a later panel:

#!/usr/bin/env bash
cat << EOF > t.txt
0 0
5 7
10 5
40 3
EOF

gmt begin
gmt figure map
  gmt set FONT_TITLE Helvetica-Bold
  gmt subplot begin 2x2 -Fs3i -M5p -A -SCb -SRl -Bwstr -R0/80/0/10
    gmt set FONT_ANNOT_PRIMARY=Times-Roman
    gmt basemap
    gmt basemap -c
    gmt set GMT_INTERPOLANT cubic
    gmt sample1d -I1 t.txt | gmt plot -W1p 
    gmt basemap -c
    gmt basemap -c --FONT_ANNOT_PRIMARY=Helvetica-Bold
   gmt sample1d -I1 t.txt | gmt plot -W1p 
   gmt subplot end
gmt end show

@joa-quim
Copy link
Member

Hm, this is what I get (no one-one command checking sorry).
And, the Times-Roman looks so much nicer in annotations then the AvantGard

map

@PaulWessel
Copy link
Member Author

I can tell we have not converged on a modern font selection. And @KristofKoch has gone quiet and not here to defend it. I will see what I need to do to strengthen the implementation but it seems straightforward. Oh, I cut/paste the last plot call after subplot end! I will edit the script next.

@PaulWessel
Copy link
Member Author

Script updated. I will work on documentation for all of this and the 11 failing tests due to "wrong" syntax.

@joa-quim
Copy link
Member

Still not filling the entire panel.
map

@PaulWessel
Copy link
Member Author

Data stops at 40, no?

@joa-quim
Copy link
Member

Yes, so it seems. Had not looked at it since first run.

@KristofKoch
Copy link
Contributor

And, the Times-Roman looks so much nicer in annotations then the AvantGard

@joa-quim beauty is in the eye of the beholder. What makes you prefer the Times-Roman font? I wrote a lengthy comment over at 3344 why I prefer a sans-serif font and backed it with the NASA Technical Report "On the typography of flight-deck documentation"; especially chapter 3.2 "Typeface (Fonts)".

Several researchers have reported that when other typographical factors are controlled, sans-serif fonts are more legible than roman.

Source: chapter 3.2 "Typeface (Fonts)", NASA-CR-177605

As @PaulWessel was unhappy with Helvetica as the default GMT font (I still prefer it) the only other sans-serif option is the Avant Garde font family. This is if we stick to the default fonts. Maybe there is the possibility to include an open source font with GMT which appeals to all/most of us?

Paul: I'm still alive - but the current COVID19 situation ties up almost all of my resources.

We had several instances wehre we set -R in a panel expecting it to carry over to othre panels.
@PaulWessel
Copy link
Member Author

I am down to two failures: ex16 and ex18. What happens here is this sequence of events:

  1. In both cases a subplot is defined. Both subplot begin calls sets the plot region.
  2. As part of panel preparations, we run commands like surface or grdmath. Neither specifies the region.
  3. While plotters can use data regions if no plot region is found, we decided that producers cannot use plot regions if no data region is given.
  4. So surface, grdmath complain they have no region.

So, as usual, there are options to choose from to fix this:

  1. We can backtrack on the requirement that producers get a data -R and use a plot region if no data region is set.
  2. We can add specific region setting to these and even move the calls outside of subplot so that they can share the -R setting (even if I added a full -R to one surface call in a panel, the next panel would also need a -R setting since they are isolated now.

This is not entirely black and white to me. In those examples, we do have plot region = data region so we would want a shared region. However, I can imagine entirely different cases where a surface call inadvertently picks up a -Rg plot region and spends forever. Here is an example from the (still pending) NASA proposal: We plan to let plotters be allowed to take input files like @earth_relief (i.e., no resolution specified) and we let GMT figure out what is the best given plot size. But we will not let producers have that capability. What the hell would grdfft @earth_relief be expected to do, for instance? So those modules need resolution. Same here I think: Leaving the data region undefined is not good, and all it takes is to specify it outside the subplot and now both plot and data regions will be set.

Agree?

@joa-quim
Copy link
Member

I'm lost. The (one) problem in ex18 is the

gmt grdmath pratt.txt POINT SDIST = mask.nc -fg

line? What is the problem of it inherits the the -R from -R@AK_gulf_grav.nc in subplot begin?
Moving this lines to before the subplot begin is one solution but it makes the script less readable since those lines move away from their context.

@PaulWessel
Copy link
Member Author

You are not lost - that is the line in ex18. The problem is one of coding - we currently do not allow a RP history to serve as RG history. However, one possibility is when a plot region is given via a data file (-R@AK_gulf_grav.nc, -Rmygrid.grd). Since those are data domains perhaps that is a reasonable rule to add? It means determining if the -R argument in RP history is a file and not w/e/s/n.

@joa-quim
Copy link
Member

@joa-quim beauty is in the eye of the beholder. What makes you prefer the Times-Roman font?

Well, @KristofKoch you are 100% right. It's in my eyes. I guess that I don't like ones being represented as T's with small arms and in which one of those arms is broken. Or zeros as squares with rounded corners.

@PaulWessel
Copy link
Member Author

PaulWessel commented Jun 24, 2020

Before I forget: We are please that @KristofKoch is still alive! And yes we understand very well what you are up against.
I mean COVID-19, not @joa-quim taste.

@joa-quim
Copy link
Member

It means determining if the -R argument in RP history is a file and not w/e/s/n.

You mean -R@AK_gulf_grav.nc would please grdmath but -R0/1/0/1 not? That would be very confusing.

@PaulWessel
Copy link
Member Author

PaulWessel commented Jun 24, 2020

Yes it would. Maybe this is such a frequently occurring situation that we should accept any -R given to subplot (and only subplot) to serve both as plot region and data region unless overridden below. I think this is the only situation where this could make sense.

@PaulWessel
Copy link
Member Author

Thinking some more about this, I think letting subplot -R set both data region and plot region makes sense. If you have another data region in mind in that context you can specify that separately on the modules before or after subplot begin. What say you @seisman? I think that would add plenty of flexibility and avoid us changing those examples as well.

@PaulWessel
Copy link
Member Author

Please comment on this @joa-quim before it is bedtime.

@joa-quim
Copy link
Member

Yes, at this time I agree that subplot -R should set both data and plot regions ... but it has been a very intense pair of days so mind changes are allowable.

@PaulWessel
Copy link
Member Author

So that handles ex18, but not ex16: In that example, we are sharing the -R (now) for gridders and plotters, but in the first panel we run gmt surface @Table_5_11.txt -I0.2 -Graws0.nc, while in another panel we run gmt surface @Table_5_11.txt -Graws5.nc -T0.5. The problem is that no -I is given to surface since the -I0.2 is only visible to a panel. So I think there is no way around this problem but to add the -I to those commands (-I is not really a common option anyway) or move the gridding to outside the start of the panels. it would be the sampe problem with any -r.

@PaulWessel
Copy link
Member Author

Just moving the first surface call to before the first panel plot fixes the problem since now all grid settings (R, I, r) are shared at the subplot level. I think this is good. I will update subplot man page to explain how this works.

Otherwise we are looking to find -R when not required or wanted.
New version is correct in that -Jx implies equal scaling so it should not scale x and y sepsrately.
@PaulWessel PaulWessel changed the title WIP Let gmt.history and gmt.conf be hierarchical and separate for panels and figures Let gmt.history and gmt.conf be hierarchical and separate for panels and figures Jun 24, 2020
@PaulWessel
Copy link
Member Author

PaulWessel commented Jun 24, 2020

I have completed this upgrade. After minor changes to some rouge scripts, all test pass as before minus the 2 known. Implementing the hierarchical scheme for history and conf was useful and found a few problems in the C code as well as bad scripting. Please check and approve, @joa-quim and @seisman . Closes #3520.

@seisman
Copy link
Member

seisman commented Jun 25, 2020

Just tried a little with this branch. I would say it works as I expect.

Here are some comments:

  1. Although we have ~800 tests, there are no tests which is designed for testing this feature. Perhaps you can add your example above as a test?
  2. It's not clear to me what history is remembered. The documentation says only common options are remembered, but from your discussions above, it seems -I is also remembered.

@PaulWessel
Copy link
Member Author

Only common options are remembered, but we learned the hard way that for grids we kind of need not only region and registration, but also increment. Since -I was never a common option (think -I in grdimage), I made grid increments behave that way in the history.
I will add that script as a test shortly.

@PaulWessel
Copy link
Member Author

Nice test of splines as well:

panels_history

Copy link
Member

@seisman seisman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@PaulWessel PaulWessel merged commit 925e272 into master Jun 26, 2020
@PaulWessel PaulWessel deleted the history-hierarchy branch June 26, 2020 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants