broadinstitute / keras-rcnn

Keras package for region-based convolutional neural networks (RCNNs)
Other
555 stars 222 forks source link

fixed ObjectDetection call() input inconsistencies #191

Closed drwaltman closed 6 years ago

drwaltman commented 6 years ago

Changes made to fix inconsistencies caused by rearranging the expected inputs to call(), as reviewed by @jorgeecardona in #164.

Moved the input definitions outside detections() so that they can be used to define bounding_boxes and scores more explicitly.

jhung0 commented 6 years ago

Thanks for the change. There are other places where this is used though, so could you make the changes to those as well (e.g. models/_rcnn.py)?

drwaltman commented 6 years ago

No problem. If I am not mistaken, I think you had already covered that change in a previous commit (models/_rcnn.py#L93). I cloned the features/preprocessing-objectdetection branch before making these changes.

jhung0 commented 6 years ago

It seems something's off because the tests are failing.

codecov-io commented 6 years ago

Codecov Report

Merging #191 into features/preprocessing-objectdetection will not change coverage. The diff coverage is 50%.

Impacted file tree graph

@@                           Coverage Diff                           @@
##           features/preprocessing-objectdetection     #191   +/-   ##
=======================================================================
  Coverage                                   75.87%   75.87%           
=======================================================================
  Files                                          28       28           
  Lines                                        1057     1057           
=======================================================================
  Hits                                          802      802           
  Misses                                        255      255
Impacted Files Coverage Δ
keras_rcnn/layers/_object_detection.py 21.56% <0%> (ø) :arrow_up:
keras_rcnn/preprocessing/_object_detection.py 86.86% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a23cb2a...aa810d9. Read the comment docs.

jhung0 commented 6 years ago

@drwaltman Oh sorry, I just realized this was for a branch. Could you make the same changes in a pull request with master as the target? (Also models/_rcnn.py will need to be edited there)

drwaltman commented 6 years ago

@jhung0 Do you want me to make another pull request to master with the same changes, or is that unnecessary since you have already merged this one to #164?

jhung0 commented 6 years ago

Please make another one

On Mar 20, 2018 8:29 PM, "drwaltman" notifications@github.com wrote:

@jhung0 https://github.com/jhung0 Do you want me to make another pull request to master with the same changes, or is that unnecessary since you have already merged this one to #164 https://github.com/broadinstitute/keras-rcnn/pull/164?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/broadinstitute/keras-rcnn/pull/191#issuecomment-374802689, or mute the thread https://github.com/notifications/unsubscribe-auth/AJJbgtRHKfPDTGqbco2mU0VqnHhMEqT2ks5tgZ7PgaJpZM4SjZMQ .