Add stereo matching losses #6554
Conversation
TeodorPoncu
added
module: datasets
module: reference scripts
new feature
and removed
module: datasets
labels
|
Also as a general comment I suggest to only keep the losses that you need to replicate the models that we will provide weights for. |
There was a problem hiding this comment.
Hi @TeodorPoncu , thanks for the PR! In general, could you add reference (url to paper or other existing implementation) for the losses?
Also, I have few comments and questions.
| if exclude_large: | ||
| # exclude invalid pixels and extremely large diplacements | ||
| flow_norm = torch.sum(flow_gt**2, dim=1).sqrt() | ||
| valid_flow_mask = valid_flow_mask & (flow_norm < max_flow) |
There was a problem hiding this comment.
In the transforms we have max_disparity and in here we have another max_flow and I think they are similar things.
Just wondering if we actually need both or just having max_flow here is enough?
There was a problem hiding this comment.
max_flow would be enough if we assume users to use our transforms. If that assumption is correct than that term is not needed.
There was a problem hiding this comment.
Looking at how sometimes we modify the valid_mask on the loss:
- We limit the norm of the flow in sequence loss with max_flow
- We limit each disparity axis to be some ratio of the image size in photometric loss
I think we may not need max_disparity in the transform. So if the dataset dont have any mask, we assume that the mask is all True during the transform. When we want to calculate the loss, then we actually modify this mask.
What do you think on this @TeodorPoncu ?
There was a problem hiding this comment.
Unfortunately, because some datasets have None returns for the disparity mask, whilst others don't. We need a transform that fills in that None with something. Even if max_disparity is not needed, we still have to impute somehow the None.
There was a problem hiding this comment.
Currently we allow max_disparity to be optional, and if it is None it will cause the problem you mention here. I think one possible solution is to let valid_flow_mask to be Optional as well here, and we treat None as if it is True everywhere. As indicated on previous comment, lets discuss on this particular topic first.
| if exclude_large: | ||
| # exclude invalid pixels and extremely large diplacements | ||
| flow_norm = torch.sum(flow_gt**2, dim=1).sqrt() | ||
| valid_flow_mask = valid_flow_mask & (flow_norm < max_flow) |
There was a problem hiding this comment.
Looking at how sometimes we modify the valid_mask on the loss:
- We limit the norm of the flow in sequence loss with max_flow
- We limit each disparity axis to be some ratio of the image size in photometric loss
I think we may not need max_disparity in the transform. So if the dataset dont have any mask, we assume that the mask is all True during the transform. When we want to calculate the loss, then we actually modify this mask.
What do you think on this @TeodorPoncu ?
| def _sequence_loss_fn( | ||
| flow_preds: List[Tensor], | ||
| flow_gt: Tensor, | ||
| valid_flow_mask: Tensor, |
There was a problem hiding this comment.
Just to follow up on my comment on #6549.
We may want the valid_flow_mask to be Optional here. But lets discuss this first
Note: I notice we already make valid_mask to be optional in Photometric loss
| if exclude_large: | ||
| # exclude invalid pixels and extremely large diplacements | ||
| flow_norm = torch.sum(flow_gt**2, dim=1).sqrt() | ||
| valid_flow_mask = valid_flow_mask & (flow_norm < max_flow) |
There was a problem hiding this comment.
Currently we allow max_disparity to be optional, and if it is None it will cause the problem you mention here. I think one possible solution is to let valid_flow_mask to be Optional as well here, and we treat None as if it is True everywhere. As indicated on previous comment, lets discuss on this particular topic first.
There was a problem hiding this comment.
Hi @TeodorPoncu , overall looks good to me! Just have a question regarding register_buffer
Summary: * Moved more losses into classes * Added photometric loss * quick fix for ssim loss return value * added references * replaced with unsqueeze * renaming variables * add ref to consistency loss * made mask optional everywhere. generalised photometric displacement * smoothness typo * fixed flow channel selection bug * aligned with training script Reviewed By: NicolasHug Differential Revision: D39765303 fbshipit-source-id: 59fb6a5e0248d126b15390f2af91c5fc6f121909 Co-authored-by: Joao Gomes <[email protected]>
Some of the Stereo Matching criterions that can be added to a training loop for regularisation purposes. Contains two candidates for more generic usage: respectively SSIM and PSNR losses