support ellipsoidal healpix and more indexing schemes through healpix-geo#192
support ellipsoidal healpix and more indexing schemes through healpix-geo#192keewis wants to merge 7 commits into
healpix-geo#192Conversation
|
Hi keewis, thank you for the PR. Since I am also at the KM-summit, I will not be able to check your PR in-depth this week, but we will probably meet at some point during the week. Maybe we should first talk about healpix-geo. From what I could find you are the maintainer of this lib, right? I think that the type-definitions you put into Healpix.vue should be rather be part of healpix-geo. I also tried to store Regarding your Are those really always the right types? E.g. from what I could see in your healpix-geo, So I am not very familiar with WASM and typical library patterns for WASM, but I could maybe do a PR next week if you need some help. |
|
Also a trivial nitpick: I would like to have the JS console clean on the production-gridlook. So I would prefer to have no |
yes, definitely
This for sure is a hack, I agree that this should not be needed in calling code (that was the intention behind problem 3 I mentioned in the initial post). The only reason it is there is to get the typing to resolve, and if you have any input for getting this right I'd be happy to take that.
That's interesting, I remember it failing when I tried it. I'd be happy to remove that call, as that removes some of the overhead I mentioned.
I didn't test
Mainly historical, For the |
|
I had another talk with Codex on this topic. Especially about the healpix-geo interface, I will just paste the output I got: I reviewed the WASM-facing API and the Gridlook call site, ignoring the math as requested. Current Shape The most serious interface problem is that Gridlook casts every scheme to one shared signature with Main Improvements
const grid = healpixGeo.createGrid({
scheme: "nested",
depth,
ellipsoid,
});
grid.vertex(cell, u, v);
grid.center(cell);
grid.cellAt(lon, lat);
grid.bitCombine(i, j);This would remove dynamic casts in Gridlook, hide the
Right now Prefer: const ellipsoid = healpixGeo.Ellipsoid.from({ semi_major_axis, inverse_flattening });
const grid = healpixGeo.createGrid({ scheme, depth, ellipsoid });Rust-side, that means exported functions should borrow a handle, not consume an enum value. The current
Gridlook calls A better public interface would expose something like: grid.fillVertices(cell, steps, outLonLat);
grid.vertices(cell, steps); // returns Float64Array or Float32Array
grid.cellsAt(lonLatArray, outCells);This keeps rendering in JS/Three, but moves repeated coordinate generation behind one WASM call.
The API takes
The generated type says type EllipsoidInput =
| { radius: number }
| { semi_major_axis: number; inverse_flattening: number }
| { semi_major_axis: number; semi_minor_axis: number };Then reserve Priority Verification: I ran |
|
Chiming in as an interested downstream user! I maintain mortie, a packed-morton HEALPix indexing spec/library — the packed words are close cousins of zuniq: left-aligned path bits, self-describing order, curve-ordered sorts. Additionally, I'm developing a writer/reader pair (zagg / moczarr) that produce sparse HEALPix zarr stores, and just started building a shard/coverage viewer on a gridlook fork. This PR is directly enabling for that work — two of its three headline features are things I'd otherwise have to hand-roll: ids past 2^53 (my stores are order-24+ where Happy to help get it there—
Is there anything specific blocking this from your side that a downstream tester/collaborator could knock out? |
Co-authored-by: Justus Magin <keewis@users.noreply.github.com>
|
thanks for the offer to help. I think if you have experience with WASM and rust it would be most helpful if you could check / review the js / ts bindings of healpix-geo to see if I'm doing anything wrong or inefficiently, and possibly we can try to replace the explicit loops in the gridlook code in this PR with building a region of data in the WASM memory and performing vectorized operations (provided that's more efficient). I also am not as sure when it comes to the way I'm building the packages: am I using the right target ( There's plenty of spherical datasets around (the example datasets, for example), so at least from that perspective this should be fine. That's not to say that testing with your data wouldn't be helpful either way. |
This adds support for ellipsoidal healpix to the healpix view, by replacing$0 <= n < 2^{53}$ )
@hcsmap/healpixwithhealpix-geo. This change in libraries also allows it to support different indexing schemes such as ring or zuniq, and means that we're not limited to the lossless range ofnumber(There's a bunch of problems right now (my javascript / typescript knowledge is pretty limited):
healpix-geocould be improved, right now I need a bunch of type overrides (but maybe that's just because I don't know as much about the type system)Either way, hopefully this helps you unblock #191