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

Remove deprecated unary/binary_function. #191

Merged
merged 1 commit into from
Dec 14, 2018

Conversation

sdebionne
Copy link
Contributor

std::unary_function and std::binary_function are deprecated in C++11 and removed in C++17. I think they are safe to remove since C++11 is required anyway.

@sdebionne
Copy link
Contributor Author

BTW, I have a question about the object functions implemented in channel_numeric_operations.hpp.

Take channel_plus_t for instance:

template <typename Channel1,typename Channel2,typename ChannelR>
struct channel_plus_t {
    ChannelR operator()(typename channel_traits<Channel1>::const_reference ch1,
                        typename channel_traits<Channel2>::const_reference ch2) const {
        return ChannelR(ch1)+ChannelR(ch2);
    }
};

Casting both ch1 and ch2 values to the type of the result channel before the addition may prevent overflow but it also prevents use cases where Channel1 is an integer type and Channel2 a floating point. Should the implementation rather be:

template <typename Channel1,typename Channel2,typename ChannelR>
struct channel_plus_t {
    ChannelR operator()(typename channel_traits<Channel1>::const_reference ch1,
                        typename channel_traits<Channel2>::const_reference ch2) const {
        return ChannelR(ch1+ch2);
    }
};

@mloskot mloskot added ext/numeric boost/gil/extension/numeric/ cat/enhancement Improvements, but not fixes addressing identified bugs labels Dec 14, 2018
@mloskot mloskot added this to To Do in C++11 Modernization via automation Dec 14, 2018
Copy link
Member

@mloskot mloskot left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mloskot
Copy link
Member

mloskot commented Dec 14, 2018

Given ChannelR = int,

int{1} + int{double{1.8}}
int{int{1} + double{1.8}}

  1. First double to int truncating conversion, then addition yields the int result.

  2. First implicit conversions for the purpose of obtaining the common real type of int{1} to double, then yields double result, finally performs truncating conversion to int.

AFAIU, the fractional part remains the same and constant, regardless if present before or after the addition, until it is discarded. So, both seem to be equivalent, unless you are thinking of a different case?

I think, generally, explicit conversion of operands makes the arithmetic cleaner/safer (e.g. avoids float = int / int bug), so I'd be inclined to keep return ChannelR(ch1)+ChannelR(ch2);. Unless I'm missing any points of the problem, then please, I'll appreciate your explanation.

/cc @stefanseefeld


p.s. Most of the times I look into the template-based arithmetic expressions in GIL, trying to analyse them in terms of conversions, promotions and ranks, I quickly get confused and lost. But, this is one of the important core topics that we need to clean up! (also to clear the flood of compilation warnings).
So, I really appreciate you raise subject, more eyes and brains FTW. I'd only suggest to open separate issue to discuss each encounter separately.

@sdebionne
Copy link
Contributor Author

Thanks for the fast reply. Actually I want to implement flatfield correction, so it's more the channel_div_t I was looking at:

  1. int{128} / int{double{0.8}}; gives div by zero
  2. int{int{128} / double{0.8}}; gives the expected result

But this is more tricky than I had thought! Opening a separate issue to continue discussing that.

@mloskot
Copy link
Member

mloskot commented Dec 14, 2018

That case 1. is exactly the division issue I was writing about in my earlier comment.

Copy link
Member

@stefanseefeld stefanseefeld left a comment

Choose a reason for hiding this comment

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

Looks good !

@mloskot mloskot merged commit 6b0b66c into boostorg:develop Dec 14, 2018
C++11 Modernization automation moved this from To Do to Done Dec 14, 2018
@mloskot
Copy link
Member

mloskot commented Dec 14, 2018

@sdebionne Thank you

@mloskot mloskot added this to the Boost 1.70 milestone Dec 16, 2018
@sdebionne sdebionne deleted the fix-dpctd-functional branch December 17, 2018 14:34
@sdebionne sdebionne mentioned this pull request Dec 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat/enhancement Improvements, but not fixes addressing identified bugs ext/numeric boost/gil/extension/numeric/
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants