earthtide bug fixes and improvement #379
Conversation
in line 38 of earthtide.rst is
```
Required Arguments
------------------
Either **-G**, **-S** or **-L**
```
but actually the exsitance of -T instead of -L is checked.
Now it also makes sense that:
gmt earthtide -L52/21
works, giving tidal displacements for current computer time.
Command:
gmt earthtide -L33/33 -T2010-01-01T/2015-01-01T00:00:02/1y
was givinig correct gregorian time stamp, but incorrect values of
Moon/Sun position and tidal components.
It is better to make this interval unit invalid, as with current
implementaion of sun_moon_track() and solid_ts() this time interval will
not be easy to implement.
The user can alway use:
for year in {2010..2015}; do
gmt earthtide -L33/33 -T${year}-01-01T
done
Also the result of two command
gmt earthtide -L33/33 -T2010-01-01T/2010-01-01T00:00:02/1s
gmt earthtide -L33/33 -T2010-01-01T/2010-01-01T00:00:02/1
has identical gregorian date output so missing interval unit is
interpreted differently by time parser and earthtide computation
routines. This patch adds consistency. As interval unit has to be
specifid explicitly as d h m or s
+n was ignored with computation of Moon/Sun position and tidal
components. Moreover it was ignored by computation, but not by output
time stamp, making big confusion. Compare output of commands:
gmt earthtide -L52/21 -T2010-01-01T/2010-01-01T01:00:00/30m
gmt earthtide -L52/21 -T2010-01-01T/2010-01-01T01:00:00/3+n
echo
gmt earthtide -S -T2010-01-01T/2010-01-01T01:00:00/30m
gmt earthtide -S -T2010-01-01T/2010-01-01T01:00:00/3+n
Consider output of commands
gmt earthtide -L52/21 -T2010-01-01T/2010-01-01T00:00:10/0.1s
gmt earthtide -L52/21 -T2010-01-01T/2010-01-01T00:00:10/1s
gmt earthtide -L52/21 -T2010-01-01T/2010-01-01T00:00:10/20+n
gmt earthtide -L52/21 -T2010-01-01T/2010-01-01T00:00:10/25+n
which are very confusing. As the first column is changing, while desired
parameters are sometimes computed correctly, and sometimes not.
This is due to problem that if ttdel2 is less
then half second then fmjd is never changed in subroutines for
computation of Moon/Sun position of tidal deformation.
after series of bug fixes, new tests are added
|
What is the status here, @joa-quim? |
|
Well, still waiting for revision of the void-returning-value issue. |
|
Where is this described? I don't see anything but I am known to suffer from blindness. |
| @@ -1219,6 +1224,11 @@ GMT_LOCAL void solid_ts(struct GMT_CTRL *GMT, struct GMT_GCAL *Cal, double lon, | |||
| tdel2 = tdel2 * T.inc; | |||
| } | |||
|
|
|||
| if (tdel2 < (0.5 / 86400) && T.n > 1){ | |||
| GMT_Report (GMT->parent, GMT_MSG_NORMAL, "Tme interval too low, must be at least 0.5 s\n"); | |||
There was a problem hiding this comment.
We cannot return a value here because the function is of void type (that's why the CI is failing for OSX). An alternative is to make tdel2 < 0.5 be 0.5 and just issue a warning that times less than 0.5 seconds are not allowed by the algorithm)
|
Shit. Week finger. It looks like I had not pushed hard enough the "submit review" button. |
|
@mrajner any news on this? |
|
@joa-quim, can we use some of the commits? It seems like there are merit in several of the things he pointed out, but that was in February. We should push this to some logical end point. Suggestions? |
|
I think we need to recreate our own version of that PR so we can make the asked modifications. |
|
OK, since this is your baby perhaps you should do that, no? |
|
Yes ... one of these days. Have to do a couple o Mirone things and realizing that I'm getting rusty on it. |
|
Yikes, that probably means it is time for me to look at the GSHHG stuff again and relearn how it works... |
* Recreate PR #379 (plus suggested fix) under our source tree. * Rearrange -T position in usage
|
Implemented in #811 |
Description of proposed changes
This series of commit deal with:
Please see extended commit message for more details.
My implementation of bug fixes may be not optimal, so devs could adapt it a little, but I hope that changes are clear enough to see the current problems.