Adding ellipsoidal equations for the Equal Earth #1101
Conversation
beuan
changed the title
src/PJ_eqearth.c
Outdated
| #include <math.h> | ||
|
|
||
| #include "projects.h" | ||
|
|
||
| PROJ_HEAD(eqearth, "Equal Earth") "\n\tPCyl., Sph."; | ||
| PROJ_HEAD(eqearth, "Equal Earth") "\n\tPCyl., Sph&Ell\n\th="; |
There was a problem hiding this comment.
What's up with the \n\th= at the end here? As far as I can tell there's no +h parameter present in this projection. Bad cut & paste job?
There was a problem hiding this comment.
Yes, bad cut & paste.
| sbeta = pj_qsfn(sin(lp.phi), P->e, 1.0 - P->es) / Q->qp; | ||
| if (fabs(sbeta) > 1) { | ||
| /* Rounding error. */ | ||
| sbeta = sbeta > 0 ? 1 : -1; |
There was a problem hiding this comment.
It would be nice to have a test case that exercises this condition.
There was a problem hiding this comment.
I am not sure I can hit this test case. The best testing case is probably phi = +/-90. In this case pj_qsfn() function returns +/-Q->qp and for some rounding error it might hit this case. Those tests are already in and looked they are passing.
There was a problem hiding this comment.
Using phi > 90 would not help, because sin function would already "fix" this.
There was a problem hiding this comment.
All right, we'll live without a test for that
|
We also need |
|
@kbevers: I added |
Indeed it was. Thanks! |
I just did. You shouldn't be able to do it. Thanks for adding this. |
|
Thanks! |
| double sbeta; | ||
| double psi, psi2, psi6; | ||
|
|
||
| if (P->es != 0.0) { |
There was a problem hiding this comment.
A little bit of nitpicking here: For clarity, I would prefer to have block-scoped comments precede the block they refer to:
/* Ellipsoidal case, converting to authalic latitude */
if (P->es != 0.0) {
sbeta = pj_qsfn(sin(lp.phi), P->e, 1.0 - P->es) / Q->qp;
/* Rounding error. */
if (fabs(sbeta) > 1)
sbeta = sbeta > 0 ? 1 : -1;
}
/* Spheroidal case, using latitude */
else
sbeta = sin(lp.phi);
Or with even less branching:
sbeta = sin(lp.phi);
/* In the ellipsoidal case, we convert sbeta to authalic latitude */
if (P->es != 0.0) {
sbeta = pj_qsfn(sbeta, P->e, 1.0 - P->es) / Q->qp;
/* Rounding error. */
if (fabs(sbeta) > 1)
sbeta = sbeta > 0 ? 1 : -1;
}
There was a problem hiding this comment.
Sorry Thomas, I merged this while you were writing...
There was a problem hiding this comment.
Don't worry - although I think my suggestion has some merit in terms of clarity, it also is mostly cosmetical and may be simply a matter of personal preference
There was a problem hiding this comment.
I happy to do the change and another PR if you would like to...
There was a problem hiding this comment.
@beuan: Well - I wrote the comment because I would prefer to minimize branching and have block-scope comments precede the block.
But it's your code, and it's already merged, so please only make the change if you personally find my (cosmetical) arguments highly convincing, and would like to see the change - in which case I will obviously feel honoured, that you found the comment convincing and/or useful :-)
There was a problem hiding this comment.
@busstoptaktik No problem. Any additional inputs are welcome.
I have changes on the way. I just need to figure out how to make a pull request without any "merge conflicts"
This changes is for #1097.