Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GeoPatch algorithm #264

Merged
7 commits merged into from
Apr 19, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 20 additions & 54 deletions lib/archethic/p2p/geo_patch.ex
Original file line number Diff line number Diff line change
Expand Up @@ -29,68 +29,34 @@ defmodule ArchEthic.P2P.GeoPatch do
end

defp compute_patch(lat, lon) do
lat_sign = sign(lat)
lon_sign = sign(lon)
lat_pos = (lat + 90) / 11.25
lon_pos = (lon + 180) / 22.5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@I-Archer-Zero
Looks like added 90,180 to avoid negative sign but look at

lat_pos

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It must be
lat_pos = (lat + 90) / 22.5

Copy link
Member Author

@Neylix Neylix Mar 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi !
Dividing lat by 22.5 will have a result between 0 and 8. But we are looking for a range between 0 and 16 to get all hexadecimal possibilities (0 to F).
So we have to divide by 11.25 to get it.

I think on issue's example there is a mismatch. On the graph, latitude only have 8 parts, but on explication bellow it says that first 2 digits are on range of 0 to F so 16 parts...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@I-Archer-Zero

Copy link
Member

@internet-zero internet-zero Mar 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Neylix (Cc @samuel-uniris)
Good observation!

lat should be divided by 22.5 and not 11.25 because we need square patches (not rectangle)
This also means that the lat/22.5 will have a result between 0 to 8

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi ! Thanks for your return.
I didn't know it had to be a square, I'll update it as you requested. Also need to add 4 to result to get index from C to 3 as in your graph.
I also noticed that in issue you say that first digit is for longitude index, but I did latitude first, should I update it too ?
I'm actually out of my PC until sunday, I'll update it at this time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello !
I fixed calculation in lastest commits

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest you can you >= and <= condition checks and fix the edge cases and also implement the algo as mentioned in #253. So, first digit and third digt can be [8,9,A,B,C,D,E,F,0,1,2,3,4,5,6,7] And second digit can be [C,D,E,F,0,1,2,3]

As precision digit being computed as per previous implementation.


fdc = [lat / 90, lon / 180]
first_digit = main_index_patch(trunc(lat_pos))
second_digit = main_index_patch(trunc(lon_pos))

sd =
[(lat - lat_sign * 45) / 2, (lon - lon_sign * 90) / 2]
|> resolve_with_sign([lat, lon])
lat_precision = ((lat_pos - trunc(lat_pos)) / 0.25) |> trunc()
lon_precision = ((lon_pos - trunc(lon_pos)) / 0.25) |> trunc()

sdc = [List.first(sd) / 22.5, List.last(sd) / 45]
third_digit = precision_index_patch(lat_precision, lon_precision)

td =
[
(List.first(sd) - lat_sign * 11.25) / 2,
(List.last(sd) - lon_sign * 22.5) / 2
]
|> resolve_with_sign(sd)

tdc = [List.first(td) / 5.625, List.last(td) / 11.25]

patch =
[index_patch(fdc), index_patch(sdc), index_patch(tdc)]
|> Enum.join("")

patch
[first_digit, second_digit, third_digit]
|> Enum.join("")
end

defp index_patch([f_i, s_i]) when f_i > 0.5 and f_i <= 1 and s_i < -0.5 and s_i >= -1, do: '0'
defp index_patch([f_i, s_i]) when f_i > 0.5 and f_i <= 1 and s_i < 0 and s_i >= -0.5, do: '1'
defp index_patch([f_i, s_i]) when f_i > 0.5 and f_i <= 1 and s_i < 0.5 and s_i >= 0, do: '2'
defp index_patch([f_i, s_i]) when f_i > 0.5 and f_i <= 1 and s_i < 1 and s_i >= 0.5, do: '3'

defp index_patch([f_i, s_i]) when f_i > 0 and f_i <= 0.5 and s_i < -0.5 and s_i >= -1, do: '4'
defp index_patch([f_i, s_i]) when f_i > 0 and f_i <= 0.5 and s_i < 0 and s_i >= -0.5, do: '5'
defp index_patch([f_i, s_i]) when f_i > 0 and f_i <= 0.5 and s_i < 0.5 and s_i >= 0, do: '6'
defp index_patch([f_i, s_i]) when f_i > 0 and f_i <= 0.5 and s_i < 1 and s_i >= 0.5, do: '7'

defp index_patch([f_i, s_i]) when f_i > -0.5 and f_i <= 0 and s_i < -0.5 and s_i >= -1, do: '8'
defp index_patch([f_i, s_i]) when f_i > -0.5 and f_i <= 0 and s_i < 0 and s_i >= -0.5, do: '9'
defp index_patch([f_i, s_i]) when f_i > -0.5 and f_i <= 0 and s_i < 0.5 and s_i >= 0, do: 'A'
defp index_patch([f_i, s_i]) when f_i > -0.5 and f_i <= 0 and s_i < 1 and s_i >= 0.5, do: 'B'

defp index_patch([f_i, s_i]) when f_i > -1 and f_i <= -0.5 and s_i < -0.5 and s_i >= -1, do: 'C'
defp index_patch([f_i, s_i]) when f_i > -1 and f_i <= -0.5 and s_i < 0 and s_i >= -0.5, do: 'D'
defp index_patch([f_i, s_i]) when f_i > -1 and f_i <= -0.5 and s_i < 0.5 and s_i >= 0, do: 'E'
defp index_patch([f_i, s_i]) when f_i > -1 and f_i <= -0.5 and s_i < 1 and s_i >= 0.5, do: 'F'

defp sign(number) when number < 0, do: -1
defp sign(number) when number >= 0, do: 1
defp main_index_patch(index) do
['8', '9', 'A', 'B', 'C', 'D', 'E', 'F', '0', '1', '2', '3', '4', '5', '6', '7']
|> Enum.at(index)
end

defp resolve_with_sign([first, second], [first2, second2]) do
defp precision_index_patch(index1, index2) do
[
do_resolve_with_sign(first, first2),
do_resolve_with_sign(second, second2)
['0', '1', '2', '3'],
['4', '5', '6', '7'],
['8', '9', 'A', 'B'],
['C', 'D', 'E', 'F']
]
end

defp do_resolve_with_sign(x1, x2) do
if sign(x1) == sign(x2) do
x1
else
x2 / 2
end
|> Enum.at(index1)
|> Enum.at(index2)
end
end
13 changes: 9 additions & 4 deletions test/archethic/p2p/geo_patch_test.exs
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
defmodule ArchEthic.P2P.GeoPatchTest do
@moduledoc """
This module defines the test case to be used by
geopatch tests.
"""

use ExUnit.Case

alias ArchEthic.P2P.GeoPatch
Expand Down Expand Up @@ -26,9 +31,9 @@ defmodule ArchEthic.P2P.GeoPatchTest do
end
end)

assert "511" == GeoPatch.from_ip({88, 22, 30, 229})
assert "500" == GeoPatch.from_ip({161, 235, 112, 33})
assert "410" == GeoPatch.from_ip({15, 62, 246, 57})
assert "266" == GeoPatch.from_ip({109, 164, 214, 168})
assert "3F7" == GeoPatch.from_ip({88, 22, 30, 229})
assert "3C9" == GeoPatch.from_ip({161, 235, 112, 33})
assert "3A6" == GeoPatch.from_ip({15, 62, 246, 57})
assert "401" == GeoPatch.from_ip({109, 164, 214, 168})
end
end