Skip to content

Load IR model from buffer#16034

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
Quantizs:irLoadFromBuffer
Dec 19, 2019
Merged

Load IR model from buffer#16034
opencv-pushbot merged 1 commit intoopencv:3.4from
Quantizs:irLoadFromBuffer

Conversation

@Quantizs
Copy link
Copy Markdown
Contributor

@Quantizs Quantizs commented Dec 2, 2019 •
edited by alalek
Loading

resolves #15578

Added implementations to load IR models from buffer

force_builders=Custom,Custom Win,Custom Mac
build_image:Custom=ubuntu-openvino-2019r3.0:16.04
build_image:Custom Win=openvino-2019r3.0
build_image:Custom Mac=openvino-2019r3.0

test_modules:Custom=dnn,python2,python3,java
test_modules:Custom Win=dnn,python2,python3,java
test_modules:Custom Mac=dnn,python2,python3,java

buildworker:Custom=linux-1
test_opencl:Custom=OFF
test_bigdata:Custom=1
test_filter:Custom=*

@Quantizs Quantizs mentioned this pull request Dec 2, 2019
Closed
dkurt
dkurt reviewed Dec 2, 2019
View reviewed changes
alalek
alalek reviewed Dec 2, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased, reworked, added test.

dkurt
dkurt reviewed Dec 2, 2019
View reviewed changes
dkurt
dkurt reviewed Dec 2, 2019
View reviewed changes
dkurt
dkurt previously approved these changes Dec 2, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@dkurt dkurt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks @Quantizs and @alalek!

@dkurt dkurt self-assigned this Dec 2, 2019
@alalek
Copy link
Copy Markdown
Member

alalek commented Dec 2, 2019

readFromModelOptimizer
readFromModelOptimizerBuffer
readFromModelOptimizerBufferPtr

Any objections about these names?

@dkurt dkurt dismissed their stale review December 3, 2019 05:34

Methods naming

@dkurt
Copy link
Copy Markdown
Member

dkurt commented Dec 3, 2019

@alalek, sorry. I missed this

dkurt
dkurt reviewed Dec 3, 2019
View reviewed changes
modules/dnn/include/opencv2/dnn/dnn.hpp Outdated
* @returns Net object.
*/
CV_WRAP static
Net readFromModelOptimizerBuffer(const std::vector<uchar>& bufferModelConfig, const std::vector<uchar>& bufferWeights);
Copy link
Copy Markdown
Member

@dkurt dkurt Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the versions should have the same name: readFromModelOptimizer and readNetFromModelOptimizer.

Copy link
Copy Markdown
Member

@alalek alalek Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same name is not good and should be avoided.
At least for bindings support.
"String" (bytes array in meaning of UTF-8 file path) and vector<uchar> (or other forms of bytes array) is similar things and bindings wrappers may convert between them.

readFromModelOptimizer should be called readFromModelOptimizerFile but it is too late.

What is about prefixes?

  • "read" for files ("load" is also used, for example in objdetect module)
  • "create" / "build" for buffers

Copy link
Copy Markdown
Member

@dkurt dkurt Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here an example when bindings work correctly with Darknet model: #11104 (comment)

Copy link
Copy Markdown
Member

@alalek alalek Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't say that they not working. My concern here is that overused overloading is not reliable.

Feel free to update patch (just keep eyes on documentation).

Copy link
Copy Markdown
Member

@dkurt dkurt Dec 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Quantizs, please change Net::readFromModelOptimizer* as well.

@Quantizs
Copy link
Copy Markdown
Contributor Author

Quantizs commented Dec 16, 2019

Hi,
When will this feature be merged ?

@dkurt
Copy link
Copy Markdown
Member

dkurt commented Dec 16, 2019

@Quantizs, As we discussed with @alalek, I'd like to recommend to rename methods in the same way: readNetFromModelOptimizer because in Python they can work fine (with bytearray). So you can fix it.

dkurt
dkurt approved these changes Dec 19, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@dkurt dkurt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks for contribution!

opencv-pushbot pushed a commit that referenced this pull request Dec 19, 2019
@alalek
Merge pull request #16034 from Quantizs:irLoadFromBuffer
4342657
@opencv-pushbot opencv-pushbot merged commit aa80f75 into opencv:3.4 Dec 19, 2019
@alalek alalek mentioned this pull request Dec 19, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alalek alalek alalek left review comments

@dkurt dkurt dkurt approved these changes

Assignees

@dkurt dkurt

Labels

category: dnn feature

Projects

None yet

Milestone

3.4.9

Development

Successfully merging this pull request may close these issues.

4 participants

@Quantizs @alalek @dkurt @opencv-pushbot