Fixed rHealpix projection, closing #1206.

[pelson]

NOTE: Targets the 5.2 branch, as this is a bug fix.

I dug into the issue in #1206 and found a patch for the original code. Because the code has been factorized subsequently, it distills down to the following simple change.

No tests yet - any guidance very welcome!

[kbevers]

Thanks for the fix. We can apply it to the 5.2 branch but that isn't going to be very helpful for future releases as I am not expecting to put out a patch release for 5.2. Please open a PR against master instead. Should the need for a 5.2.1 release arise I'll cherry-pick all bug fixes into the 5.2 branch.

Regarding tests, you can add them here: https://github.com/OSGeo/proj.4/blob/ac6f0021a3ce6110e5a0a917aec9c0c614443e84/test/gie/builtins.gie#L1911-L1954

Just add another section with your new test coordinates and a bit of text explaining what you are testing.

[pelson]

Thanks for the hint @kbevers. Changed target to master (w. rebase).
Will add some tests now.

[kbevers]

Will add some tests now.

Nice. It would be great if you can tests that has a few coordinates in each square of the projection. Hopefully we can avoid similar regressions in the future then.

[pelson]

Was having problems with make install on master....

make check
Making check in include
Making check in proj
Making check in internal
make[3]: Nothing to be done for `check'.
make[3]: Nothing to be done for `check-am'.
make[2]: Nothing to be done for `check-am'.
Making check in src
make[1]: *** No rule to make target `cs2cs.c', needed by `cs2cs.o'.  Stop.
make: *** [check-recursive] Error 1

Ended up just building it (to get the error above) then running make check-local in test/gie/.

[kbevers]

Maybe you need to re-run autogen.sh and configure before building. There's some changes between 5.2 and current master.

Ended up just building it (to get the error above) then running make check-local in test/gie/.

Does that mean you've created some new tests?

[pelson]

Does that mean you've created some new tests?

It does. Unfortunately, I've also found more bugs with both the rHealpix and the Healpix projections. They are characterised by the following images, and I've tested them all the way back to v4.4.9 (the oldest version I could build easily) [edit: I was mis-testing the versions, so don't have confidence in that finding).

But at least the basic rhealpix is actually fixed... 😄

I noticed these when writing tests and realising that I was getting the same projected x coordinate for things that should be translated onto different cube faces. For example, I wasn't expecting the following to pass:

+-------------------------------------------------------------------------------
+operation +proj=rhealpix   +south_square=2  +north_square=3
+-------------------------------------------------------------------------------
+tolerance 0.1 mm
+accept  45 -50
+expect  -15011332.016655975953   -5802815.142163854092
+accept  135 50
+expect  -15011332.016655975953   5802815.142163855955

[pelson]

Confirming that this is now behaving as expected for all square locations of the rHealpix, and also tested the full extent (forward & inverse) of the triangle healpix projection.

Output with cartopy:

[kbevers]

Confirming that this is now behaving as expected for all square locations of the rHealpix, and also tested the full extent (forward & inverse) of the triangle healpix projection.

Great! So this PR is ready for merging now, yes?

[pelson]

Great! So this PR is ready for merging now, yes?

Confirmed. The images above were produced with the changes from this PR, the existing (limited) tests pass, as do the additional tests, so merge away if happy 👍

[mboeringa]

Hi,
Just for reference, I am running into an issue that is likely caused by this:

The data you see here from OpenStreetMap was reprojected from lat/long Geometry storage in PostgreSQL 11.2/PostGIS 2.5.1 to rhealpix, then generalized, and subsequently back-transformed to lat/long Geometry.

Considering the strange cut up just over Tasmania and New Zealand, I suspect this is the same issue, as that line corresponds with the envelope of the rhealpix projection. Or am I overlooking another potential issue?

In Ubuntu, I see "proj-bin 4.9.3-2" listed as cartographic projection library.

[mboeringa]

I've also found more bugs with both the rHealpix and the Healpix projections.

@pelson. As I ran into an issue with rhealpix, likely caused by this bug as I wrote in the post above, can you elaborate a bit about healpix? Was the healpix projection also affected by this regression in the v4.9.1+ proj libraries, or not? Your current statement leaves me guessing whether or not I can safely switch to healpix instead of rhealpix. If both projections were affected, switching projections would not make sense.

[mboeringa]

Well, don't know if this is the whole story, but doing a test with the same Geofabrik extract and using Healpix instead of rHealpix, seems to preliminary indicate that at least Healpix is still working in the v4.9.3-2 library:

Would appreciate some formal confirmation though... and I need to do some further testing with other parts of the globe.

[pelson]

Considering the strange cut up just over Tasmania and New Zealand, I suspect this is the same issue, as that line corresponds with the envelope of the rhealpix projection. Or am I overlooking another potential issue?

Looks very likely that it is the same issue.

Your current statement leaves me guessing whether or not I can safely switch to healpix instead of rhealpix. If both projections were affected, switching projections would not make sense.

healpix and rhealpix aren't the same (polyhedral) projection, so "safely" is a matter of context. With regards to the bug that was introduced, my comments definitely suggest it did impact both "rHealpix" and plain-old "healpix" (we should call it something like tHealpix, since one is rectangles and the other triangles...), but the extent of the code-changes suggests that a fix was only needed in the rHealpix case. There was quite a lot of churn of PJ_healpix.c between v4 and master (v6) and I seem to recall the bug was fixed independently (sorry, no reference to hand).

The pictures I attached do show the extent of the issue well though, so if you know your region of interest then it may be that switching to healpix is a good choice.

Sorry I don't have any more detail than that to hand.

[mboeringa]

Hi @pelson

Appreciate your response, thanks.

Unfortunately, as I understand it now, the issue with the healpix projection will affect me as well, looking at this image you posted of the problem if that is accurate as you state, as I need global coverage:

https://user-images.githubusercontent.com/810663/50390569-84d6b900-0730-11e9-842d-308ece1993e1.png

So it is clear I will need to wait for an update to PostGIS to include the fix to proj4, or Ubuntu if proj4 is installed by default in Ubuntu and doesn't come with PostGIS... I am not to familiar with this Ubuntu stuff and all these different components like proj4 and their release cycles.

Do you potentially have any idea when this change might trickle down to PostGIS and / or Ubuntu?

[pelson]

Do you potentially have any idea when this change might trickle down to PostGIS and / or Ubuntu?

Sorry I have no idea. I'd imagine you'll be talking multiple months. The change will be part of the v6.0 release of Proj4, which is due imminently (1st March is the schedule). It is plausible that there will need to be some work downstream to be able to use the new release, and then Ubuntu will need to package it and ensure that all tools using Proj4 are compatible with the new release. In essence - quite a bit of work.

Truth be told, if you are using Ubuntu, you might find you can get hold of an old version of Proj4 that doesn't exhibit the bug (v4.9.1 according to my bug report). This is probably the quickest way of "fixing" the issue you are seeing.

[mboeringa]

@pelson

Thanks for the insights. I will need to contemplate other options then.