Skip to content
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

[Enhance] Adds windows compilation support #551

Closed
wants to merge 1 commit into from

Conversation

divyanshj16
Copy link

@divyanshj16 divyanshj16 commented May 17, 2021

fixes #169
Here is the summary of the fixes,

  • In windows' compiler variables cannot be passed in the size of initialization of the array, so we have to initialize it something like this. int *xx= new int[variable] - source
  • The long datatype in windows and linux are different so the following source suggested it to convert everything to long long .source
  • While defining EPS, its scope had to be changed to __device__, otherwise it was throwing errors about undefined EPS.

@CLAassistant
Copy link

CLAassistant commented May 17, 2021

CLA assistant check
All committers have signed the CLA.

@Wuziyi616
Copy link
Contributor

@divyanshj16 Hi, we really appreciate your contribution, and this PR seems to be very helpful for us. Can you kindly sign the CLA above and then we will start to review your modifications?

@divyanshj16
Copy link
Author

@Wuziyi616 I signed the CLA

@Wuziyi616
Copy link
Contributor

@ZwwWayne Can you have a look at this PR?

@Wuziyi616
Copy link
Contributor

The unit test failed. Seems to because the modified GPU ops can't run successfully on Linux platform?

@Tai-Wang Tai-Wang changed the title adds windows compilation support [Enhance] Adds windows compilation support May 19, 2021
@divyanshj16
Copy link
Author

Almost all of them says operation cancelled. Could it be because of something else?

@Wuziyi616
Copy link
Contributor

If one run failed in CI, all other runs will be canceled. I think we need to find out the reason in that failed run. The error traceback is:

Traceback:
/opt/hostedtoolcache/Python/3.6.13/x64/lib/python3.6/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/test_data/test_datasets/test_dataset_wrappers.py:4: in <module>
    from mmdet3d.datasets.builder import build_dataset
mmdet3d/datasets/__init__.py:3: in <module>
    from .custom_3d import Custom3DDataset
mmdet3d/datasets/custom_3d.py:9: in <module>
    from ..core.bbox import get_box_type
mmdet3d/core/__init__.py:2: in <module>
    from .bbox import *  # noqa: F401, F403
mmdet3d/core/bbox/__init__.py:4: in <module>
    from .iou_calculators import (AxisAlignedBboxOverlaps3D, BboxOverlaps3D,
mmdet3d/core/bbox/iou_calculators/__init__.py:1: in <module>
    from .iou3d_calculator import (AxisAlignedBboxOverlaps3D, BboxOverlaps3D,
mmdet3d/core/bbox/iou_calculators/iou3d_calculator.py:5: in <module>
    from ..structures import get_box_type
mmdet3d/core/bbox/structures/__init__.py:1: in <module>
    from .base_box3d import BaseInstance3DBoxes
mmdet3d/core/bbox/structures/base_box3d.py:5: in <module>
    from mmdet3d.ops.iou3d import iou3d_cuda
mmdet3d/ops/__init__.py:12: in <module>
    from .knn import knn
mmdet3d/ops/knn/__init__.py:1: in <module>
    from .knn import knn
mmdet3d/ops/knn/knn.py:4: in <module>
    from . import knn_ext
E   ImportError: /home/runner/work/mmdetection3d/mmdetection3d/mmdet3d/ops/knn/knn_ext.cpython-36m-x86_64-linux-gnu.so: undefined symbol: _ZNK2at6Tensor8data_ptrIxEEPT_v

I am not very familiar with CUDA. I will let my teammates look into this problem.

@divyanshj16
Copy link
Author

divyanshj16 commented May 31, 2021

@Wuziyi616, @ZwwWayne Is there any update on this?

@ZwwWayne
Copy link
Collaborator

Hi @divyanshj16 ,
Thanks for your kind contribution. Could resolve the conflict first? The CI fails due to a weird reason, I just reran it and am waiting for the outcome.

@divyanshj16
Copy link
Author

divyanshj16 commented Jul 23, 2021

Looks like the conflicts are addressed in different PR. Apologies for delayed response.

@Wuziyi616
Copy link
Contributor

@divyanshj16 No worry! Thank you for pointing us to this solution. Since we have opened a new PR to solve this issue, I will close this PR.

@Wuziyi616 Wuziyi616 closed this Jul 23, 2021
tpoisonooo pushed a commit to tpoisonooo/mmdetection3d that referenced this pull request Sep 5, 2022
* fix ncnn test in regression test

* update doc

* fix docstring
@OpenMMLab-Coodinator
Copy link

Hi @divyanshj16 !We are grateful for your efforts in helping improve this open-source project during your personal time.
Welcome to join OpenMMLab Special Interest Group (SIG) , where you can share your experiences, ideas, and build connections with like-minded peers.

To join the SIG channel, simply message moderator— OpenMMLab on Discord or briefly share your open-source contributions in the #introductions channel and we will assist you. Look forward to seeing you there! Join us :https://discord.gg/UjgXkPWNqA

If you have a WeChat account,welcome to join our community on WeChat. You can add our assistant :openmmlabwx. Please add "mmsig + Github ID" as a remark when adding friends:)
Thank you again for your contribution❤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

compilation error C2131: expression did not evaluate to a constant
5 participants