Skip to content

added basic support for CV_16F (the new datatype etc.)#12463

Merged
vpisarev merged 4 commits intoopencv:masterfrom
vpisarev:fp16_basic
Sep 10, 2018
Merged

added basic support for CV_16F (the new datatype etc.)#12463
vpisarev merged 4 commits intoopencv:masterfrom
vpisarev:fp16_basic

Conversation

@vpisarev
Copy link
Copy Markdown
Contributor

@vpisarev vpisarev commented Sep 7, 2018

CV_USRTYPE1 is now equal to CV_16F, which may break some [rarely used] functionality. We'll see. The following functions and methods should support CV_16F:

  • Mat::copyTo()
  • Mat::setTo()
  • Mat::convertTo()
  • RNG::fill()
  • split(), merge() and most other functions that only need to know the element size to process it, not the content.
  • norm()
  • arithm and logic functions (do we need it?)
  • print() (need to test it)
  • persistence (XML, YAML, JSON I/O)
docker_image:Custom=ubuntu:18.04
CPU_BASELINE:Custom=AVX2

…s now equal to CV_16F, which may break some [rarely used] functionality. We'll see
…ch importer (need to find a better solution)
#define CV_32F 5
#define CV_64F 6
#define CV_USRTYPE1 7
#define CV_16F 7
Copy link
Copy Markdown
Member

@alalek alalek Sep 8, 2018

Choose a reason for hiding this comment

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

#define CV_USRTYPE1 7

Lets leave message for users about compatibility/migration problems:

#define CV_USRTYPE1 (void)"CV_USRTYPE1 support has been dropped in OpenCV 4.0"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

there are a few places where USRTYPE1 is still used, but need to be adjusted, e.g. Torch importer in DNN.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

New size of these elements are 2 bytes instead of previous 8.
This will lead to memory corruption.

/cc @dkurt

CV_Assert(!_mat.empty());

if (_mat.empty())
return;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Early error detection: #8300

Filling empty Mat with random numbers is not a normal use case of OpenCV.

case CV_64F: *os << "64F"; break;
case CV_USRTYPE1: *os << "USRTYPE1"; break;
case CV_USRTYPE1: *os << "16F"; break;
default: *os << "INVALID_TYPE"; break;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is time to replace this switch to something of:

depthToString(t)
typeToString(t)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the naive change to typeToString(t) breaks the perf tests because of different naming conventions. Let's leave it as-is for now

@vpisarev vpisarev self-assigned this Sep 10, 2018
@vpisarev
Copy link
Copy Markdown
Contributor Author

👍

@vpisarev vpisarev merged commit 6d7f587 into opencv:master Sep 10, 2018
@vpisarev vpisarev deleted the fp16_basic branch September 12, 2018 12:24
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
* added basic support for CV_16F (the new datatype etc.). CV_USRTYPE1 is now equal to CV_16F, which may break some [rarely used] functionality. We'll see

* fixed just introduced bug in norm; reverted errorneous changes in Torch importer (need to find a better solution)

* addressed some issues found during the PR review

* restored the patch to fix some perf test failures
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.

2 participants