helium / blockchain-etl

Blockchain follower that follows and stores the Helium blockchain
Apache License 2.0
64 stars 37 forks source link

incorrect location_hex #191

Closed radrob1 closed 3 years ago

radrob1 commented 3 years ago

Describe the bug Hotspot has incorrect location_hex based on GPS coords.

name: clean-fleece-salamander address: 112BQH7acxEH5j3tVSRUQMBQhWZjsdYXLMuU2Qt5jhFcaNXKm2MW location coords: 45.79972505148626, 16.020780778758787 current location_hex via API: 881e130dc9fffff

Expected behavior Expected location_hex based on both python h3 and js h3: 881e130dc1fffff

Additional Context Could be lingering from #190 backfill process (according to @madninja)

madninja commented 3 years ago

I need to look at this in more detail.. AFAICT the erlang h3 library is not returning the same value.

On ETL:

h3:to_string(h3:from_geo(h3:to_geo(h3:from_string("8c1e130dc8d9dff")), 8)).
"881e130dc9fffff"
madninja commented 3 years ago

@radrob1 could you share the code you used for the expected location_hex calculation?

radrob1 commented 3 years ago

@madninja Very basic using the lat,lng from the hotspots API.

Python

h3.geo_to_h3(45.79972505148626, 16.020780778758787,8)
'881e130dc1fffff'

If I use the same technique as above:

h3.geo_to_h3(h3.h3_to_geo('8c1e130dc8d9dff')[0],h3.h3_to_geo('8c1e130dc8d9dff')[1],8)
'881e130dc1fffff'

I also noticed the lat/lng returned in the API are not the res12 center coords which may be the issue somewhere. API lat/lng: 45.79972505148626,16.020780778758787 res12 h3_to_geo: 45.79972505148625, 16.020780778758795

madninja commented 3 years ago

@radrob1 and what kind of OS/architecture are you running that on?

radrob1 commented 3 years ago

MacOS M1

Just ran the same on x86 Amazon Linux and got 881e130dc9fffff so now looks like an issue with M1 arch.

madninja commented 3 years ago

MacOS M1

Just ran the same on x86 Amazon Linux and got 881e130dc9fffff so now looks like an issue with M1 arch.

No I get the same result as you on an Intel MBP. I think it's differing libm implementations that roll floating point errors in differently :-/. Not much we can do about that at this point.

radrob1 commented 3 years ago

I see. The question I am wrestling with now is which location is used for PoC. Based on the reward scale of this hex (different from what we have been looking at above) visualized in explorer:

https://explorer-beta.helium.com/hotspots/hex/881968adedfffff

Those two hotspots scales should be 0.5 based on HIP17 rules, but they are both 1.0.

radrob1 commented 3 years ago

@madninja Got another example of incorrect location hex based on reward scale: https://explorer-beta.helium.com/hotspots/hex/881f40480bfffff https://explorer-beta.helium.com/hotspots/hex/881f404809fffff

The hotspots in those two hexes have a reward scale of 0.5 so they must be in the same res8 hex based on HIP17. If you use the location coords to get res12, then h3ToParent for res8, these two hotspots would be in 881f404809fffff together and thus have the 0.5 reward scale.

I think we originally thought parent hexes were determined using the location coords, but it appears that it is determined using the h3ToParent function using res12 location.

Using javascript (this is what helium.place is now doing) h3ToParent(geoToH3(lat,lon, 12), 8)

low-taffy-owl-res8
madninja commented 3 years ago

So are you suggesting we revert back to the getParent approach vs the gyrations we went through here: https://github.com/helium/blockchain-etl/issues/189

cc @kent-williams for background and asking for his input

radrob1 commented 3 years ago

Unfortunately, it appears that way. I am still trying to figure out why the original way using getParent would have lead to https://github.com/helium/explorer/issues/472

But in my testing with helium.place and looking at the reward scales, getParent(res12_location, 8) seems to be the correct approach as I don't see the above issue when using it.