psmeca: Make the last few columns of input optional #2018

seisman · 2019-11-09T20:36:55Z

Fixes #2016.

PaulWessel

Maybe add a comment before the Set_Columns line to remind us:
/* Because psmeca reads optional columns we cannot insist records are all the same, hence VAR below */

seisman · 2019-11-09T20:41:45Z

I have a few failing tests locally. Need to check them

PaulWessel · 2019-11-09T20:51:34Z

OK, let me know.

seisman · 2019-11-09T21:03:41Z

@PaulWessel This one doesn't work. It seems the last column "10km" is read as a float value "10.0" instead of a string.

gmt psmeca -R239/240/34/35.2 -JX8c -Baf -Sa2c << END > test2.ps
239.384 34.556 12.   180     18   -88  5 0 0 10km
END

PaulWessel · 2019-11-09T21:06:24Z

I may be able to fix that since "km" is not a valid unit, but I can imagine some users thinking that any string can be used for that title, and if they give 00101 then we will read that as 101 and not as a string. This is the problem of allowing variable record lengths with or without a string at the end...

seisman · 2019-11-09T21:34:33Z

I may have a working solution. We can still use GMT_COL_FIX but only read Ctrl->S.n_cols - 2 columns (i.e. the required columns). Then the remaining columns if any are saved in In->text.
Then use sscanf(In->text, "%lf %lf %[^\n]s\n", &xynew[ix], &xynew[iy], event_title); to read the newX, newY and title from In->text. Do you think it's a good way?

PaulWessel · 2019-11-09T22:17:56Z

Yes we can do that. In fact, in my longer posting earlier I was about to write that scheme but decided to see if the VAR would be enough. But it cannot tell the trailing text. I am adding a minor improvement to the parsing (it will check if the trailing chars after the last digit or single period in a string is more than one or (if one) does not match a known unit, then it is deemed text. But as mentioned this will not catch strings that look like numbers. So you suggestion is more foolproof. I will still commit my fix since it applies in general and will not parse 10km as a number.

PaulWessel · 2019-11-09T22:26:13Z

If you want to try the above, note that you will need to do this:

n_scanned = scanf (In->text, "%s %s %[^\n]s\n", Xstring, Ystring, event_title); 
if (n_scanned >= 2) { /* Got new x,y coordinates */
    if (ix == GMT_X) {
       gmt_scanf_arg (GMT, Xstring, GMT_IS_LON, false, &xynew[GMT_X]);
       gmt_scanf_arg (GMT, Ystring, GMT_IS_LAT,  false, &xynew[GMTY]);
   }
    else {
       gmt_scanf_arg (GMT, Ystring, GMT_IS_LON, false, &xynew[GMT_Y]);
       gmt_scanf_arg (GMT, Xstring, GMT_IS_LAT,  false, &xynew[GMT_X]);
    }
}

To handle proper parsing of 135:45W etc.

PaulWessel · 2019-11-09T22:39:37Z

If you want to take a first crack at this then I can help with any pesky details.

seisman · 2019-11-09T22:43:52Z

OK. I'll look into it later.

PaulWessel · 2019-11-09T23:03:03Z

Maybe something like this?. This assumes you may give an event title but not new x,y

if (In->text) {	/* Must examine the trailing text */
	n_scanned = scanf (In->text, "%s %s %[^\n]s\n", Xstring, Ystring, event_title);
	if (n_scanned >= 2) { /* Got new x,y coordinates and possibly event title */
		if (ix == GMT_X) {	/* Expect lon lat */
			gmt_scanf_arg (GMT, Xstring, GMT_IS_LON, false, &xynew[GMT_X]);
			gmt_scanf_arg (GMT, Ystring, GMT_IS_LAT,  false, &xynew[GMTY]);
		}
		else {	/* Expect lat lon */
			gmt_scanf_arg (GMT, Ystring, GMT_IS_LON, false, &xynew[GMT_Y]);
			gmt_scanf_arg (GMT, Xstring, GMT_IS_LAT,  false, &xynew[GMT_X]);
		}
		if (n_scanned == 2)	/* Got no title */
			event_title[0] = '\0';
	}
	else if (n_scanned == 1)	/* Only got event title */
		strncpy (event_title, In->text, GMT_BUFSIZ-1);
	else	/* Got no title */
		event_title[0] = '\0';
}

seisman · 2019-11-10T00:46:54Z

@PaulWessel I followed your first solution, as the second one doesn't work if event title contains several whitespaces (e.g. Right lateral strike, Right and lateral will be read as the newX and newY).

I also added a test to check psmeca with different input columns. Now all tests pass.

PaulWessel · 2019-11-10T00:56:42Z

How do you handle the difference between

lon lat title

and

A longer title

both will give n_scanned 3, no?

seisman · 2019-11-10T01:00:23Z

I don't have a clever way to handle that, so I assume the newX and newY must be given if event title is needed. It's also consistent with older GMT4 or GMT5 behavior.

PaulWessel · 2019-11-10T01:03:36Z

OK, I guess a clever solution would check if the resulting xynew coordinates are NaNs or not, and if they both are then change event_title to be the whole trailing text, e.g.

if (gmt_M_is_dnan (xynew[GMT_X]) and gmt_M_is_dnan (xynew[GMT_Y]))
	strncpy (event_title, In->text, GMT_BUFSIZ-1);

seisman · 2019-11-10T01:28:22Z

For a title like "10 km", gmt_scanf_arg returns 10.0000 and 0.000, not NaN. Why?

PaulWessel · 2019-11-10T01:31:13Z

Odd, this is what I was fixing with #2020 and it caught the 10km title from before.

seisman · 2019-11-10T01:34:31Z

A title "Strike slip" also gives me 0.0 and 0.0, not NaN.

PaulWessel · 2019-11-10T01:52:33Z

I have trouble switching to this branch - it complains about conflicts etc... I would approve and then try your case with 3 word title?

seisman · 2019-11-10T01:54:20Z

Try these commands?

# delete your local  meca-varcolumns branch
git branch -D meca-varcolumns
# switch to this branch
git checkout meca-varcolumns

PaulWessel · 2019-11-10T02:05:09Z

OK now. Sorry, was in middle of building the full docs. Please give me a command or two that fails so I can debug.

seisman · 2019-11-10T02:09:16Z

gmt psmeca -R239/240/34/35 -JX4c -Ba0 -Sa1c > meca.ps << EOF
239.384 34.556 12.   180     18   -88  5 10 km
EOF

The title is "10 km".

PaulWessel · 2019-11-10T02:16:58Z

The FIX cols set 9 cols, and your data record has 9 including the title. isn't the title in addition to the required cols? IF 10 km is the title then there should be 6 cols only?

PaulWessel · 2019-11-10T02:18:33Z

Probably never mind. Xcode does not wipe and rebuild the supplements when the source changes, so I need to rebuild the whole thing. Your code seems to do 6 but my debugger says 9...

If GMT_IS_NAN is returned we must set to NaN manually.

seisman · 2019-11-10T03:38:37Z

@PaulWessel Your fix works well except that newX and newY need to be reset to 0.0 if they're NaN.

I've made the fix and updated the test script.

seisman · 2019-11-10T03:47:44Z

I still have a question. If -: is used, the first two columns are "lat lon". What about the newX and newY columns? I expect to give "newY newX", but the script below doesn't work:

gmt psmeca -R239/240/34/35 -JX8c -Baf -Sa1c -C1p -: -pdf meca << EOF
34.556 239.384 12.   180     18   -88  5 34.8 239.8 10 km
EOF

I have to give "newX newY". i.e. This works:

gmt psmeca -R239/240/34/35 -JX8c -Baf -Sa1c -C1p -: -pdf meca << EOF
34.556 239.384 12.   180     18   -88  5 239.8 34.8  10 km
EOF

PaulWessel · 2019-11-10T03:53:53Z

It should by lat lon in both places I think. I suggest we change the if test on ix to be

if (GMT->current.setting.io_lonlat_toggle[GMT_IN]) {

else
<do the first situation where X is X

which is cleaner than the ix stuff.

PaulWessel · 2019-11-10T03:56:35Z

Also, the checking of xynew versus zero is no good. The day may come when you want to give 0,0 as the new coordinates and then you cannot. Please do this instead:

Initialize the xynew array to GMT->session.d_NaN
Remove the reassignment of xynew back to zero
Change the test near L768 to use the gmt_M_is_dnan macro

seisman · 2019-11-10T04:09:13Z

I made the change aa7fa4b to fix the issue with -:. Please help check if it's correct.

Also, the checking of xynew versus zero is no good. The day may come when you want to give 0,0 as the new coordinates and then you cannot. Please do this instead:
Initialize the xynew array to GMT->session.d_NaN
Remove the reassignment of xynew back to zero
Change the test near L768 to use the gmt_M_is_dnan macro

I agree, but it's used for a long time, and I have seen many scripts using 0 0 as newX and newY.

PaulWessel · 2019-11-10T04:11:10Z

Looks good to me.

PaulWessel · 2019-11-10T04:13:25Z

You mean people will place 0 0 when they use -C but don't want another placement instead of not giving the coordinates? Maybe we could handle it under compatibility and say this will break in GMT 7 and warn if we get 0,0 instead of NaN NaN until then?

seisman · 2019-11-10T04:17:24Z

You mean people will place 0 0 when they use -C but don't want another placement instead of not giving the coordinates?

Yes, this is exactly what the GMT4 manpage says:

Entries in these columns are necessary with the -C option. 
Using 0,0 in columns 8 and 9 will plot the beach ball at the longitude, latitude 
given in columns 1 and 2. The −: option will interchange the order of columns (1,2) and (8,9).

PaulWessel · 2019-11-10T04:20:39Z

Of, a simpler time when NaNs were just bread eaten by Indians...
I think we can now handle just leaving those coordinates off entirely. Alternatively accept NaN NaN to mean no new location. And then until GMT 7 warn about 0,0. The test on zero is not good and GMT-like...

seisman · 2019-11-10T04:30:05Z

Just realize that it's a bad idea to allow "title"-only input.

The Global CMT project (the most used focal mechanism solution data source) returns:

-176.96 -29.25 48 7.68 0.09 -7.77 1.39 4.52 -3.26 26 X Y 010176A

Our current implementation gives

It means breaking many people's results.

PaulWessel · 2019-11-10T05:21:29Z

On the road. Certainly we can handle that case? Paul Wessel, Professor and Chair Dept. of Earth Sciences (formerly Geology & Geophysics) SOEST, U of Hawaii at Manoa

…

On November 9, 2019 at 6:30:06 PM, Dongdong Tian ***@***.***) wrote: Just realize that it's a bad idea to allow "title"-only input. The Global CMT project (the most used focal mechanism solution data source) returns: -176.96 -29.25 48 7.68 0.09 -7.77 1.39 4.52 -3.26 26 X Y 010176A Our current implementation gives [image: image] <https://user-images.githubusercontent.com/3974108/68538828-90cb6980-0348-11ea-8340-d48e189a20ca.png> It means breaking many people's results. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2018?email_source=notifications&email_token=AGJ7IXZGPLPDVAIPNH62WSDQS6E45A5CNFSM4JLJOPI2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDUVC7Q#issuecomment-552161662>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGJ7IX3AQVQPQIWW6H2COQLQS6E45ANCNFSM4JLJOPIQ> .

The Global CMT project (the most used focal mechanism solution data source) returns records like -176.96 -29.25 48 7.68 0.09 -7.77 1.39 4.52 -3.26 26 X Y 010176A

seisman · 2019-11-10T23:35:34Z

@carltape This PR fixes the psmeca input issue reported in #2016. It would be good if you can give it a try.

In the new implementation, the new coordinates and event_title are optional. For example, -Sa now can accept following input formats:

longitude latitude depth strike dip rake mag
longitude latitude depth strike dip rake mag newX newY
longitude latitude depth strike dip rake mag event_title
longitude latitude depth strike dip rake mag newX newY event_title

carltape · 2019-11-14T22:31:39Z

@seisman I installed this branch and confirm that it resolves the issue you were working on: psmeca allows variable number of columns, as before. Thank you!

In the process of wider testing, I found a lingering issue with psmeca. I will reopen #661

seisman · 2019-11-15T16:30:02Z

@carltape Thanks for your reports and tests! This PS has been merged into 6.0 branch.

PaulWessel requested changes Nov 9, 2019

View reviewed changes

seisman force-pushed the meca-varcolumns branch from caeb619 to 2d5c9e6 Compare November 9, 2019 23:46

seisman added 2 commits November 9, 2019 19:37

psmeca: Make the last few columns of input optional

8e2dc6e

Add a test for different input columns

cf77595

seisman force-pushed the meca-varcolumns branch from 7d75d6a to cf77595 Compare November 10, 2019 00:43

Remove the wrong comment

5afedb0

Give title only

d7b8f7b

PaulWessel and others added 3 commits November 9, 2019 16:37

Needed to check return codes

2ed4e84

If GMT_IS_NAN is returned we must set to NaN manually.

Revert newX and newY to zero if not given

a9cf942

Update test script

426faf5

PaulWessel approved these changes Nov 10, 2019

View reviewed changes

Fix columns for -:

aa7fa4b

PaulWessel and others added 3 commits November 9, 2019 20:50

Allow X Y string for placeholders in meca input

88618e7

The Global CMT project (the most used focal mechanism solution data source) returns records like -176.96 -29.25 48 7.68 0.09 -7.77 1.39 4.52 -3.26 26 X Y 010176A

Add test for GCMT XY placeholder

7f24f50

Update description of meca -S

f0f795d

seisman added this to the 6.0.1 milestone Nov 10, 2019

Merge branch '6.0' into meca-varcolumns

c4114bd

seisman added 2 commits November 14, 2019 19:41

Merge branch '6.0' into meca-varcolumns

c03caa5

Update psmeca input format description

e526ae5

seisman merged commit 0fa52fb into 6.0 Nov 15, 2019

seisman deleted the meca-varcolumns branch November 15, 2019 16:29

This was referenced Nov 15, 2019

psmeca: Last few columns of input data should be optional #2016

Closed

pscoupe: Make the last few columns of input data optional #2065

Merged

psmeca: Make the last few columns of input optional #2018

psmeca: Make the last few columns of input optional #2018

Conversation

seisman commented Nov 9, 2019 • edited Loading

PaulWessel left a comment

Choose a reason for hiding this comment

seisman commented Nov 9, 2019

PaulWessel commented Nov 9, 2019

seisman commented Nov 9, 2019

PaulWessel commented Nov 9, 2019

seisman commented Nov 9, 2019

PaulWessel commented Nov 9, 2019

PaulWessel commented Nov 9, 2019

PaulWessel commented Nov 9, 2019

seisman commented Nov 9, 2019

PaulWessel commented Nov 9, 2019

seisman commented Nov 10, 2019

PaulWessel commented Nov 10, 2019

seisman commented Nov 10, 2019

PaulWessel commented Nov 10, 2019

seisman commented Nov 10, 2019 • edited Loading

PaulWessel commented Nov 10, 2019

seisman commented Nov 10, 2019

PaulWessel commented Nov 10, 2019

seisman commented Nov 10, 2019

PaulWessel commented Nov 10, 2019

seisman commented Nov 10, 2019

PaulWessel commented Nov 10, 2019

PaulWessel commented Nov 10, 2019

seisman commented Nov 10, 2019 • edited Loading

seisman commented Nov 10, 2019 • edited Loading

PaulWessel commented Nov 10, 2019

PaulWessel commented Nov 10, 2019

seisman commented Nov 10, 2019

PaulWessel commented Nov 10, 2019

PaulWessel commented Nov 10, 2019

seisman commented Nov 10, 2019 • edited Loading

PaulWessel commented Nov 10, 2019

seisman commented Nov 10, 2019

PaulWessel commented Nov 10, 2019 via email

seisman commented Nov 10, 2019

carltape commented Nov 14, 2019

seisman commented Nov 15, 2019

seisman commented Nov 9, 2019 •

edited

Loading

seisman commented Nov 10, 2019 •

edited

Loading

seisman commented Nov 10, 2019 •

edited

Loading

seisman commented Nov 10, 2019 •

edited

Loading

seisman commented Nov 10, 2019 •

edited

Loading