Skip to content

Add division operators for Matx.#13928

Merged
alalek merged 2 commits intoopencv:3.4from
catree:add_matx_div_operations
Feb 21, 2020
Merged

Add division operators for Matx.#13928
alalek merged 2 commits intoopencv:3.4from
catree:add_matx_div_operations

Conversation

@catree
Copy link
Copy Markdown
Contributor

@catree catree commented Feb 27, 2019 •
edited by alalek
Loading

Add the following operations for Matx:

    {
        Matx31d P1(1, 2, 3);
        P1 /= 2;
        std::cout << "P1: " << P1.t() << std::endl;
    }
    {
        Matx31d P1(1, 2, 3);
        P1 /= 2.0f;
        std::cout << "P1: " << P1.t() << std::endl;
    }
    {
        Matx31d P1(1, 2, 3);
        P1 /= 2.0;
        std::cout << "P1: " << P1.t() << std::endl;
    }
    {
        Matx31d P1(1, 2, 3);
        P1 = P1 / 2;
        std::cout << "P1: " << P1.t() << std::endl;
    }
    {
        Matx31d P1(1, 2, 3);
        P1 = P1 / 2.0f;
        std::cout << "P1: " << P1.t() << std::endl;
    }
    {
        Matx31d P1(1, 2, 3);
        P1 = P1 / 2.0;
        std::cout << "P1: " << P1.t() << std::endl;
    }
    {
        Matx31d P1(1, 2, 3);
        std::cout << "P1: " << (P1 / 2).t() << std::endl;
    }
    {
        Matx31d P1(1, 2, 3);
        std::cout << "P1: " << (P1 / 2.0f).t() << std::endl;
    }
    {
        Matx31d P1(1, 2, 3);
        std::cout << "P1: " << (P1 / 2.0).t() << std::endl;
    }

Output:

P1: [0.5, 1, 1.5]
P1: [0.5, 1, 1.5]
P1: [0.5, 1, 1.5]
P1: [0.5, 1, 1.5]
P1: [0.5, 1, 1.5]
P1: [0.5, 1, 1.5]
P1: [0.5, 1, 1.5]
P1: [0.5, 1, 1.5]
P1: [0.5, 1, 1.5]

TODO:

  • Add unit tests (point me to the file where I can add the corresponding tests)

alalek
alalek reviewed Feb 28, 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.

You can add test here (preferred way is adding a separate test for that).

There are already ".div()" methods (per-element, I found with Matx only, looks like scalars are not supported). Should we extend them?

There are already multiplication operation "*".
Users can write: P1 *= 0.5f instead of P1 /= 2 (div by integer is questionable anyway - unclear, not well defined - see example below).

In "integer" case there are different results:

{
    Matx<int, 3, 1> P1(1, 2, 3);
    P1 /= 2;
    std::cout << "P1: " << P1.t() << std::endl;
}
{
    Matx<int, 3, 1> P1(1, 2, 3), P2;
    P2 = P1 / 2;
    std::cout << "P2: " << P2.t() << std::endl;
}

modules/core/include/opencv2/core/matx.hpp Outdated
}

template<typename _Tp, int m, int n> static inline
Matx<_Tp, m, n>& operator /= (Matx<_Tp, m, n>& a, int alpha)
Copy link
Copy Markdown
Member

@alalek alalek Feb 28, 2019

Choose a reason for hiding this comment

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

Matx usually makes sense (used) with floating point numbers (float/double).
So "int" scalar looks out of scope here. Do we really need it?

For floating point computations we can replace multiple divisions by multiplications (they are still more effective than divisions):

_Tp alpha_inv = (_Tp)1.0f / alpha;
for( int i = 0; i < m*n; i++ )
        a.val[i] = a.val[i] * alpha_inv;

saturate_cast() is not used for floating point numbers too.

@asmorkalov asmorkalov added the pr: needs test New functionality requires minimal tests set label Jan 17, 2020
@asmorkalov asmorkalov added the Hackathon https://opencv.org/opencv-hackathon-starts-next-week/ label Jan 24, 2020
@VadimLevin
Copy link
Copy Markdown
Contributor

VadimLevin commented Jan 30, 2020 •
edited by asmorkalov
Loading

Things to be done to complete this PR:

  • Leave only floating point part of the added functionality (Avoid saturate_cast usage for floating point types)
  • Add unit tests (test file: opencv/modules/core/test/test_operations.cpp)

@asmorkalov asmorkalov removed the pr: needs test New functionality requires minimal tests set label Feb 21, 2020
@asmorkalov asmorkalov force-pushed the add_matx_div_operations branch from cc9510e to c87b99e Compare February 21, 2020 07:09
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Feb 21, 2020

@alalek I finalized the PR as it was discussed on the meeting. Please take a look and merge.

alalek
alalek approved these changes Feb 21, 2020
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.

Thank you!

@alalek alalek merged commit 966c219 into opencv:3.4 Feb 21, 2020
@alalek alalek mentioned this pull request Feb 26, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alalek alalek alalek approved these changes

Assignees

@alalek alalek

Labels

category: core Hackathon https://opencv.org/opencv-hackathon-starts-next-week/

Projects

None yet

Milestone

3.4.10

Development

Successfully merging this pull request may close these issues.

4 participants

@catree @VadimLevin @asmorkalov @alalek