Fix boundingbox transformation #591
Conversation
| return self.__class__( | ||
| point1=self.minpoint.warp(spaceobj), | ||
| point2=self.maxpoint.warp(spaceobj), | ||
| space=spaceobj, | ||
| ) | ||
| return self.corners.warp(spaceobj).boundingbox |
There was a problem hiding this comment.
Since boundingbox of a pointset shifts the min and max points 1e-6m, this change causes the boundingbox to expand 1 voxel in each direction. (see line 245-257 in pointset.py).
I suggest either:
- actually finding the min and max corner points to create bounding box or,
- Removing the offset the bounding box of a pointset has
There was a problem hiding this comment.
any idea why the shift was introduced in PointSet? I would say it should be fixed there if we do not have obvious reasons.
There was a problem hiding this comment.
Unfortunately, it was there before I arrived and we never discussed it so I do not know. If I were to guess, to make sure the point is enclosed by the bounding box. But I think this shouldn't be necessary.
There was a problem hiding this comment.
@AhmetNSimsek The offset is for avoiding the case of a zero-volume bounding boxes with strict 2D shape, which would have no intersection with any other objects and could therefore not be used for filtering points or finding intersecting images. I think we can remove the numerical offset, but raise a warning whenever a zero volume bounding box is constructed, because this should hardly happen. For example, bounding boxes of 2D images must have nonzero volume if the 2D image lives in 3D space, and the boundingbox function of images needs to cover that. The depth is the pixel size then. I assume that zero volume bounding boxes mostly occurred from manual construction of annotations in some tutorials, and this minimum size has been introduced to overcome the resulting problems, but it is not a clean solution. My suggestion is to remove the offset, raise an exception when construction a zero-volume bounding box, then run all tests and tutorial notebooks to see when that happens. Ultimatlely the exception can become a warning.
…due to masking (data) issue" This reverts commit 99e140b.
|
The image feature e2e tests are failing because some warped corners of a bounding box can return None which causes the warping to create a bbox with (nan, nan, nan) points and such a bbox has no intersection with any well-defined bbox. I'll see how I can circumvent the issue. |
siibra had a serious bug when transforming bounding boxes: Both
.warpand.transformwould create the new bounding box by just transforming the min- and max point. But since the bounding box is axis aligned, this is not correct - other corner points could form the new min- or maxpoint in the new coordinate system after warping. Therefore, all corner points (8 in general) need to be transformed, and a new min- and maxpoint have to be determined in the target space to create the transformed bounding box.