You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Hello, I have been reading the code multiple times trying to make it faster. However, the first step is to understand it properly before proposing any change.
In particular, this line 1239 seems to contain a bug. I'll copy above the current code to exaplain why is it wrong, from my point of view:
defdecode_netout(self, netout, anchors, obj_thresh, net_h, net_w):
grid_h, grid_w=netout.shape[:2]
nb_box=3# (1): From this line, netout is a numpy array of 4 dimentionsnetout=netout.reshape((grid_h, grid_w, nb_box, -1))
nb_class=netout.shape[-1] -5boxes= []
netout[..., :2] =self._sigmoid(netout[..., :2])
netout[..., 4:] =self._sigmoid(netout[..., 4:])
netout[..., 5:] =netout[..., 4][..., np.newaxis] *netout[..., 5:]
netout[..., 5:] *=netout[..., 5:] >obj_threshforiinrange(grid_h*grid_w):
row=i/grid_w# small comment: using '//' instead of '/' makes it unnecesary to cast results to int every time in the future (just replace float operation by its int version) col=i%grid_wforbinrange(nb_box):
# 4th element is objectness score# (2): because of (1), the nextline will let objectness as a single np.float valueobjectness=netout[int(row)][int(col)][b][4]
# (3): the following line contains the errorif (objectness.all() <=obj_thresh): continue# Do more thingspassreturnboxes
Here goes the long explanation on the error of (3):
because of (2) objectness.all() will return always True, except for the case where objectness is 0, (exactly zero) in which case it will return False.
Then, when <= operator is applied, when .all() evaluates to False, it is casted to integers as 0, as obj_thresh usually is a value in range (0..1) (0.5 by default), so this evaluates to if 1 <= 0.3: continue, and therefore, every single value is being taken.
basically what is happening is that we are only skipping those cases in which objectness is exactly zero, and keep al the others.
In addition to this, I refer to the original implementation provided by @OlafenwaMoses and it is a little bit different (and correct) from my point of view.
question: is there a reason for such strange behavior? may the same issue be replicated in other methods?
I'll write a PR soon to change it, but would like to know @OlafenwaMoses view on the issue
Thanks
The text was updated successfully, but these errors were encountered:
Thanks very much for this thorough review @rola93 . During the implementation for training custom YOLOv3 models, there were so many things to figure out as I raise against the time I promised in #8 , which was the 2nd deadline for delivery. Some of this things were:
providing a seamless, simple experience for the training
providing sufficient methods to evaluate trained models
compatibility in previous and new YOLOv3 code.
I admit code optimization wasn't fully implemented at the time, and even after the release, documentation and tutorials took a lot of time as well.
I will review the PR and evaluate the changes. Thanks very much once again @rola93
Hello, I have been reading the code multiple times trying to make it faster. However, the first step is to understand it properly before proposing any change.
In particular, this line 1239 seems to contain a bug. I'll copy above the current code to exaplain why is it wrong, from my point of view:
Here goes the long explanation on the error of (3):
because of (2)
objectness.all()
will return alwaysTrue
, except for the case whereobjectness
is 0, (exactly zero) in which case it will returnFalse
.Then, when
<=
operator is applied, when.all()
evaluates toFalse
, it is casted to integers as 0, asobj_thresh
usually is a value in range (0..1) (0.5 by default), so this evaluates toif 1 <= 0.3: continue
, and therefore, every single value is being taken.basically what is happening is that we are only skipping those cases in which
objectness
is exactly zero, and keep al the others.In addition to this, I refer to the original implementation provided by @OlafenwaMoses and it is a little bit different (and correct) from my point of view.
question: is there a reason for such strange behavior? may the same issue be replicated in other methods?
I'll write a PR soon to change it, but would like to know @OlafenwaMoses view on the issue
Thanks
The text was updated successfully, but these errors were encountered: