search

pyg-team / pytorch_geometric

Graph Neural Network Library for PyTorch
https://pyg.org
MIT License
20.94k stars 3.61k forks source link

Divide by Zero in SamplePoints #1988

Open Hafplo opened 3 years ago

Hafplo commented 3 years ago

🐛 Bug

SamplePoints in sample_points.py uses pos.max() instead of pos.abs().max() When all the points are negative, the maximum can be zero, causing division by zero. that produces NaNs to all positions.

To Reproduce

Steps to reproduce the behavior:

  1. set a point cloud with non-negative positions
  2. run SamplePoints

Expected behavior

  1. explain why divide my pos_max is needed (scaling? maximum of 1?)
  2. divide by max of absolute or move positions to have 0 mean.

Environment

Additional context

In my example I am placing the origin (0, 0, 0) based on some intrinsic properties of the point cloud (instead of just setting pos.mean() to be the origin)

rusty1s commented 3 years ago

Thanks, sounds good :) Fixed in master.

Hafplo commented 3 years ago

@rusty1s Amazed by the super quick response.

I still don't understand why this division is needed in the first place. Maybe @Yannick-S you can explain?

Spooky-0 commented 3 years ago

Hi, Its been a loooong time since I looked at any of this. I went through my code changes to sample_points.py and found this comment

https://github.com/rusty1s/pytorch_geometric/commit/305b5f635d4f628ed5b76ecdaa51817621c058a9#diff-f1265895070f224f0f05b33e6a035e49de97bc5a34fbfd96a9daeeb123cbdc2e

when models are too big the following operations will give 'inf',
   crashing the program. Thus we normalize the model, and later scale all
   the sampled points back to the original size.

The problem was, if i remember correctly, that different modeling software gives models of vastly different scales. If you don't do some sort of normalization the points are either millimeters apart or kilometers. I had issues of inf, as i wrote back then. The points get converted back to their original size at the end of the function, thus this just fixes the intermediate 'infinity' issues.

I don't remember exactly where the infinity occurred, but if i were to guess its during division of area / area.sum()