Features/region proposal network losses small fixes #55
Conversation
|
@hgaiser |
keras_rcnn/models.py
Outdated
| @@ -30,15 +30,15 @@ def __init__(self, inputs, features, heads, rois): | |||
|
|
|||
| rpn_prediction = keras.layers.concatenate([rpn_classification, rpn_regression]) | |||
|
|
|||
| proposals = keras_rcnn.layers.object_detection.ObjectProposal(rois)([im_info, rpn_regression, rpn_classification]) | |||
| anchors = keras_rcnn.layers.object_detection.ProposalTarget()([rpn_classification, gt_boxes, im_info]) | |||
| proposals = keras_rcnn.layers.object_detection.ObjectProposal(rois)([im_info, rpn_regression, rpn_classification]) | |||
There was a problem hiding this comment.
Remove the object_detection part of this and the next line
|
Yeah with these changes I got a successful model summary (not really the same as compiling I guess). |
| return (None,), (None, 4) | ||
| return [(None,), (None, 4)] | ||
|
|
||
| def compute_mask(self, inputs, mask=None): |
There was a problem hiding this comment.
It is trying to call output_mask for all outputs. I'm not sure why it isn't required for a single output, but for multiple outputs it seems to be required (keras-team/keras#3061). Perhaps something we should try to fix in Keras.
keras_rcnn/models.py
Outdated
| @@ -30,15 +30,15 @@ def __init__(self, inputs, features, heads, rois): | |||
|
|
|||
| rpn_prediction = keras.layers.concatenate([rpn_classification, rpn_regression]) | |||
|
|
|||
| proposals = keras_rcnn.layers.object_detection.ObjectProposal(rois)([im_info, rpn_regression, rpn_classification]) | |||
| anchors = keras_rcnn.layers.object_detection.ProposalTarget()([rpn_classification, gt_boxes, im_info]) | |||
| proposals = keras_rcnn.layers.ObjectProposal(rois)([im_info, rpn_regression, rpn_classification]) | |||
There was a problem hiding this comment.
Maybe it's not necessary, but it'd be clearer to have maximum_proposals=rois instead of just rois
Also, ObjectProposal and ProposalTarget have im_info in different orders...we should be consistent. Could you switch it so that im_info comes at the end?
There was a problem hiding this comment.
I agree. Will you make this change or should I?
|
Are you able to train? |
jhung0
merged commit cf404e8
into
broadinstitute:features/region-proposal-network-losses
|
No I didn't try training yet (are the loss layers used already?). I only got a seemingly logical model summary. |
This PR fixes some small issues that caused the model not to compile.
In addition I noticed that all inputs are shaped including their batch dimension. I wasn't completely aware of this and I'm not sure the current implementation handles it correctly (ie. it might assume a shape of (None, 4) while it is actually (None, None, 4)). @jhung0 @0x00b1 are you aware of this?