-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix the use of linspace for numpy 1.18 #354
Conversation
The change looks good. Accepting a floating point value for the |
@@ -503,8 +503,8 @@ def setDetParams(self): | |||
self.imgIds = [] | |||
self.catIds = [] | |||
# np.arange causes trouble. the data point on arange is slightly larger than the true value | |||
self.iouThrs = np.linspace(.5, 0.95, np.round((0.95 - .5) / .05) + 1, endpoint=True) | |||
self.recThrs = np.linspace(.0, 1.00, np.round((1.00 - .0) / .01) + 1, endpoint=True) | |||
self.iouThrs = np.linspace(.5, 0.95, int(np.round((0.95 - .5) / .05)) + 1, endpoint=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tempting to rewrite this as
self.iouThrs = np.linspace(.5, 0.95, int(np.round((0.95 - .5) / .05)) + 1, endpoint=True) | |
self.iouThrs = np.arange(10, 19+1) / 20 |
Read as "between 10/20 and 19/20, inclusive"
Or If you prefer hundredths:
self.iouThrs = np.linspace(.5, 0.95, int(np.round((0.95 - .5) / .05)) + 1, endpoint=True) | |
self.iouThrs = np.arange(50, 95+1, 5) / 100 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean np.arange(10, 19+1) / 20
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than that, I agree with your suggestion to use some form of arange
instead of the floating point shenanigans in np.round((0.95 - .5) / .05)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the typo above, also for the hundredths case - good catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Best use floats in the division for Python 2.7 compatibility.
Somebody please integrate this ASAP!! Until this change is merged, pycocotools will cause a crash for both TensorFlow and PyTorch object detection during the evaluation phase of training!! It took me many hours of troubleshooting to work out that backdating to Numpy 1.17.4 was the only currently available resolution. See: |
@pdollar do you plan update pycocotools package on pypi (https://pypi.org/project/pycocotools/)? |
Has this been integrated? The pytorch tutorials dont work and I am wondering which version of pycocotools to upgrade to |
For the moment the easiest solution is to make everything current, except |
What is preventing cocoapi from being fixed? I will downgrade numpy for now, but Numpy 1.18 was released months ago |
The pull request to cocoapi was merged in, but it seems the apt-get package has not been updated per this change so you still have to back to Numpy 1.17.4 for the moment. |
It was already fixed two months ago. No one should use cocoapi from apt-get or pip install or conda because cocoapi was never officially released on those places. The correct way to install cocoapi is UPDATE: |
The reality of the situation is that most people will use |
@CDahmsCellarEye This didnt work for me, pip install -U "git+https://github.com/cocodataset/cocoapi.git#subdirectory=PythonAPI" Failed with the error ``cl : Command line error D8021 : invalid numeric argument '/Wno-cpp'` |
I did see #51 but that's a workaround for a bug unfixed since 2017. All other libraries I have seen so far work with pip install .... and don't require you to download / modify source before it would run. I don't usually use python as much but its been harder to get pycocotools to work than even C/C++ libraries so far. |
Has this already been fixed? I still get the same error. |
I still get this error in May 2020. pycocotools with numpy 18.4: loading annotations into memory...
Done (t=0.38s)
creating index...
index created!
Loading and preparing results...
DONE (t=5.31s)
creating index...
index created!
Traceback (most recent call last):
File "/usr/local/lib/python3.6/dist-packages/numpy/core/function_base.py", line 117, in linspace
num = operator.index(num)
TypeError: 'numpy.float64' object cannot be interpreted as an integer
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "test.py", line 257, in <module>
opt.augment)
File "test.py", line 214, in test
cocoEval = COCOeval(cocoGt, cocoDt, 'bbox')
File "/usr/local/lib/python3.6/dist-packages/pycocotools/cocoeval.py", line 76, in __init__
self.params = Params(iouType=iouType) # parameters
File "/usr/local/lib/python3.6/dist-packages/pycocotools/cocoeval.py", line 527, in __init__
self.setDetParams()
File "/usr/local/lib/python3.6/dist-packages/pycocotools/cocoeval.py", line 507, in setDetParams
self.iouThrs = np.linspace(.5, 0.95, np.round((0.95 - .5) / .05) + 1, endpoint=True)
File "<__array_function__ internals>", line 6, in linspace
File "/usr/local/lib/python3.6/dist-packages/numpy/core/function_base.py", line 121, in linspace
.format(type(num)))
TypeError: object of type <class 'numpy.float64'> cannot be safely interpreted as an integer. |
@ppwwyyxx I've already gathered that. The problem is that I've installed the latest version of pycocotools from pip and source and I'm still getting this error on numpy 1.20. Any advice? |
Most likely you're not using what you "installed". See https://ppwwyyxx.com/blog/2019/On-Environment-Packaging-in-Python/ for some possible reasons. |
The original code cannot run under latest numpy 1.18:
This fix addresses this issue. The issue is originally reported at facebookresearch/detectron2#580
cc @rbgirshick @agrimgupta92: lvis seems to have the same issue