Glow: deduplicateConstants assert will always fire if Constant contains NaN

[geoffberry]

The assert referenced below will always assert for Constants that contain a NaN since such constants will always return false from isEqual. Furthermore, Constants that contain NaN will not be de-duplicated for this same reason. Perhaps it would be better to avoid using isEqual in ConstsEqDedup and instead do a bit-wise comparison.

This assert fires intermittently in the test GraphOptz.ReshapeConstantOneUse depending on whether the uninitialized input tensor contains a NaN.

https://github.com/pytorch/glow/blob/25ce2de7248d7be1e8e43ded9b36d5e3ce0e7cb2/lib/Optimizer/GraphOptimizer/GraphOptimizer.cpp#L1818

[jfix71]

Thanks for reporting this issue @geoffberry! Are you interested in putting up a fix for this?

[bertmaher]

Nice catch! I kind of think that assert is problematic; iirc a vanilla std::map<float> will fail to find NAN keys too. Idk if it’s worth adding special logic to dedup constants with NAN but I’m kind of ok either way.

[chadrosier]

I'm going to start working on a patch shortly.

[chadrosier]

Fixed here: https://github.com/pytorch/glow/commit/f0c08f92477e64c4fb03bb80281e6afb2ba6ffd2

[jfix71]

Thanks @chadrosier!