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.

Sign up for GitHub

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

Jump to bottom

[darknet] ready for yolo_v4 #11037

Merged
merged 13 commits into from
Aug 7, 2020
Merged

[darknet] ready for yolo_v4 #11037

merged 13 commits into from
Aug 7, 2020

Conversation

cenit
Copy link
Contributor

@cenit cenit commented Apr 26, 2020 •
edited
Loading

update the darknet port to get ready for Yolo v4 :)

old note, now fixed:

at the moment, some large files hosted on GDrive are commented because GDrive itself does not let you download files from CLI with normal syntax, so advanced users should download them manually after installation.

The required structure for CLI download should be

wget --load-cookies /tmp/cookies.txt "https://docs.google.com/uc?export=download&confirm=$(wget --quiet --save-cookies /tmp/cookies.txt --keep-session-cookies --no-check-certificate 'https://docs.google.com/uc?export=download&id=FILEID' -O- | sed -rn 's/.*confirm=([0-9A-Za-z_]+).*/\1\n/p')&id=FILEID" -O FILENAME && rm -rf /tmp/cookies.txt

where FILEID can be extracted from the commented download paths (which is the working one in a browser) and FILENAMEs are written in the snippets. Similarly, SHA512SUMs are correct even if commented out. The snippet works on bash but I cannot do it in a portable way (I don't know an equivalent Powershell snippet for now).

Since the YoloV4 public release already happened, I didn't want to stop the update here only because of secondary files.

Any suggestion?

JackBoosY
JackBoosY suggested changes Apr 27, 2020
View reviewed changes
ports/darknet/portfile.cmake Outdated Show resolved Hide resolved
@JackBoosY
Copy link
Contributor

Is the file to be downloaded large?

@cenit
Copy link
Contributor Author

cenit commented Apr 27, 2020

They are between 120MB and 260 MB each. Unfortunately those hosted on gdrive have the problems described above. I still wanted to put the links in the portfile, so that advanced users have all the informations ready.
I will try to discuss upstream also for a different hosting mechanism

@cenit
Copy link
Contributor Author

cenit commented Apr 27, 2020

@JackBoosY upstream we already changed the hosting mechanism. Now problematic downloads are managed through GitHub release. I think the PR should be ready for merge

@cenit
Copy link
Contributor Author

cenit commented Apr 27, 2020

The error in CI seems unrelated

@JackBoosY
Copy link
Contributor

@cenit Yes, they are not related.

@JackBoosY JackBoosY mentioned this pull request Apr 28, 2020
Merged
@cenit cenit closed this Apr 29, 2020
@cenit cenit reopened this May 1, 2020
@cenit
Copy link
Contributor Author

cenit commented May 5, 2020

@JackBoosY please let me know if this pr can be merged or requires other modifications

@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY
Copy link
Contributor

JackBoosY commented May 6, 2020 •
edited
Loading

@cenit Let me solve the llvm:x64-linux regression first.
And I thinkg this PR is ready to merge.
LGTM.

@JackBoosY JackBoosY added the requires:testing Needs tests added before merging label May 6, 2020
@JackBoosY
Copy link
Contributor

Testing...

@JackBoosY
Copy link
Contributor

JackBoosY commented May 20, 2020 •
edited
Loading 

CMake Error: The following variables are used in this project, but they are set to NOTFOUND.
Please set them or make sure they are set and tested correctly in the CMake files:
CUDA_cublas_LIBRARY (ADVANCED)
    linked by target "opencv_cudev" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/cudev
    linked by target "opencv_core" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-fb9e10326a/modules/core
    linked by target "opencv_cudaarithm" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/cudaarithm
    linked by target "opencv_cudaarithm" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/cudaarithm
    linked by target "opencv_flann" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-fb9e10326a/modules/flann
    linked by target "opencv_hdf" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/hdf
    linked by target "opencv_imgproc" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-fb9e10326a/modules/imgproc
    linked by target "opencv_ml" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-fb9e10326a/modules/ml
    linked by target "opencv_phase_unwrapping" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/phase_unwrapping
    linked by target "opencv_plot" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/plot
    linked by target "opencv_quality" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/quality
    linked by target "opencv_reg" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/reg
    linked by target "opencv_surface_matching" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/surface_matching
    linked by target "opencv_cudafilters" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/cudafilters
    linked by target "opencv_cudaimgproc" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/cudaimgproc
    linked by target "opencv_cudawarping" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/cudawarping
    linked by target "opencv_dnn" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-fb9e10326a/modules/dnn
    linked by target "opencv_features2d" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-
...
521dd00bd2/modules/cudaoptflow
    linked by target "opencv_stereo" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/stereo
    linked by target "opencv_stereo" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/stereo
    linked by target "opencv_superres" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/superres
    linked by target "opencv_superres" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/superres
    linked by target "opencv_videostab" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/videostab
    linked by target "opencv_videostab" in directory G:/11037/vcpkg/buildtrees/opencv4/src/4.1.1-521dd00bd2/modules/videostab

Any ideas? I think this issue is not related to this PR.

@JackBoosY
Copy link
Contributor

the x64-windows-static regression is gone.

@cenit
Copy link
Contributor Author

cenit commented May 20, 2020

Any ideas? I think this issue is not related to this PR.

did you rebuilt everything from scratch? Unfortunately the cmake 3.17.2 upgrade should trigger a full rebuild also on users, so the "abi check" proposal I saw somewhere to determine update might become very relevant!
Please delete everything and rebuild from scratch also opencv! In case you still have the issue please let me know

@JackBoosY
Copy link
Contributor

@cenit Rebuild all related ports using cmake 3.17.2 result: same error.

@cenit
Copy link
Contributor Author

cenit commented May 21, 2020

which triplet was that? Also which features combination were you testing? Looks like a combinations of OpenCV with cuda and darknet without, is it right?

Can you please try this PR on top of #11130? I suppose that PR fixes also this problem. In that case, we will have to wait also for #11130 before merging this :(

@JackBoosY
Copy link
Contributor

@cenit Building darknet[opencv-base,opencv-cuda,weights,weights-train], I will try that PR later.

@JackBoosY JackBoosY added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Jun 11, 2020
@JackBoosY JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jun 22, 2020
@cenit cenit mentioned this pull request Jun 22, 2020
Merged
@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY
Copy link
Contributor

JackBoosY commented Jul 21, 2020 •
edited
Loading

Will re-test features after #11130 merge.

@JackBoosY JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Jul 28, 2020
@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY
Copy link
Contributor

Testing...

@JackBoosY
Copy link
Contributor

JackBoosY commented Aug 3, 2020 •
edited
Loading

Pass all features tests in x86-windows, x64-windows, x64-windows-static.

@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed requires:testing Needs tests added before merging labels Aug 3, 2020
@cenit
Copy link
Contributor Author

cenit commented Aug 3, 2020

wonderful news.
I hope it can be merged soon :)

@JackBoosY
Copy link
Contributor

Ping @strega-nil for merge this PR.

@strega-nil
Copy link
Contributor

Thanks @cenit (and @JackBoosY of course!)

@strega-nil strega-nil merged commit 80fae15 into microsoft:master Aug 7, 2020
@cenit cenit deleted the dev/cenit/darknet_yolov4 branch August 24, 2020 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JackBoosY JackBoosY JackBoosY approved these changes

Assignees

@JackBoosY JackBoosY

Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cenit @JackBoosY @strega-nil