Open MoHawastaken opened 2 years ago
Hi,
No, it should just be you have to change the input dimension size to 102, by default its 78 to match the input size in the paper, but I think the example is outdated right now. I'll update the docs soon for it
Hey, you are right of course, thanks for the quick reply!
A very unrelated question. The paper says:
"The initial edge features are the positions of the lat/lon nodes connected to each icosahedron node. These positions are provided in a local coordinate system that is defined relative to each icosahedron node."
You set cos(distance), sin(distance) as edge_attributes in encoder and decoder, should that not rather be the relative position (lon_original - lon_icosa, lat_original - lon_icosa). I agree that adding distance also makes sense, but maybe without cos/sin? My simple logic: The smaller the distance, the more related the points.
Yeah, but as that distance in the sin and cos is the great circle distance from the icosahedron node to the lat/lon point, I think it is quite similar. The issue with just subtracting lat/lon points from each other is that the physical distance between two points changes if they are near the poles or the equator, and so I think the cos/sin helps to fix that issue, in a similar way that in the NormalizedMSELoss we include the cosine of the latitude to correct for each pixel becoming physically larger as we move from the poles to the equator.
An option could be added to just use the great circle distance, instead of the sin/cos part as well.
Yes, direct differences were a bad suggestion. But in order to get full 2d information in polar coordinate style, besides a distance you would also like to have a direction. cos/sin of distance does not add any intrinsic 2 dimensionality. Here is a suggestion to calculate such an angle, that is invariant to the base icosahedron grid point and is maybe a bit over the top...: 1) Find the geodesic rotation of the icosa. point to a fixed point, say (0,0,1), via Rot = T R_n(alpha) T^-1 as described in the answer of G Cab here: https://math.stackexchange.com/questions/114107/determine-the-rotation-necessary-to-transform-one-point-on-a-sphere-to-another 2) Apply the rotation to the point in the original grid. 3) So that the angle refers to North-South-West-East directions irrespective of the base icosahedron grid point, simply rotate along (0,0,1) by the angle -lat_icosa. 4) To determine the angle, project the rotated point via the normal vector (0,0,1) onto (smth,smth,1). Now you can view (0,0,1) as the origin (0,0,0) and normalize the projected point (smth,smth,0). Then (smth,smth,0) * (1,0,0) yields the cosine of the angle we are interested in.
Of course, going north from a grid point on the northern hemisphere has a different effect than going north on the southern hemisphere, so the value of such an angle is questionable for GNNs, which need invariance with respect to the base grid point, I agree...
Ah yeah, that makes sense, I guess one different way could be just decompose the distance into the north/south change and the west/east change, not in lat/long but in the great circle distance that is currently being used. Might be a bit simpler than this approach, although if you want to write a PR for this, I'd be happy to review it/merge it!
@all-contributors please add @MoHawastaken for bug
@peterdudfield
I've put up a pull request to add @MoHawastaken! :tada:
Hey, I just installed your package and all requirements, tried to run your first example usage and got the following error:
Is this a mistake on my side?
Best, Moritz