Skip to content

Use fmin and fmax for HIP environment#1116

Merged
unnonouno merged 1 commit intocupy:masterfrom
okuta:fix-min-max
Jul 16, 2018
Merged

Use fmin and fmax for HIP environment#1116
unnonouno merged 1 commit intocupy:masterfrom
okuta:fix-min-max

Conversation

@okuta
Copy link
Copy Markdown
Member

@okuta okuta commented Apr 9, 2018

This PR is related to #1094 and #1117 .

numpy.fmax complies with std c fmax function. But, min behaves same as fmin in CUDA. It is a little strange.

HIP doesn't have such behavior. So, this PR fix it.

@okuta okuta added cat:enhancement Improvements to existing features to-be-backported Pull-requests to be backported to stable branch labels Apr 9, 2018
@okuta okuta mentioned this pull request Apr 9, 2018
11 tasks
@unnonouno
Copy link
Copy Markdown
Contributor

unnonouno commented Apr 14, 2018

Sorry, I can't understand its background. Please write why this fix is required.

@unnonouno unnonouno self-assigned this Apr 14, 2018
@okuta okuta removed the to-be-backported Pull-requests to be backported to stable branch label Apr 15, 2018
@okuta
Copy link
Copy Markdown
Member Author

okuta commented Apr 15, 2018

OK, I removed to-be-backported label.

@okuta
Copy link
Copy Markdown
Member Author

okuta commented Jul 1, 2018

I resolved conflict.
Jenkins, test this please.

@chainer-ci
Copy link
Copy Markdown
Member

Jenkins CI test (for commit 1f10bbc) failed with status FAILURE.
(For contributors, please wait until the reviewer confirms the details of the error.)

@okuta
Copy link
Copy Markdown
Member Author

okuta commented Jul 2, 2018

jenkins, test this please.

@chainer-ci
Copy link
Copy Markdown
Member

Jenkins CI test (for commit 4e33247) succeeded without errors!

@okuta
Copy link
Copy Markdown
Member Author

okuta commented Jul 2, 2018

@unnonouno Test was passed. Cloud you review this?

@okuta
Copy link
Copy Markdown
Member Author

okuta commented Jul 15, 2018

numpy.fmax complies with std c fmax function. But, min behaves same as fmin in CUDA. It is a little strange.

HIP doesn't have such behavior. So, this PR fix it.

@unnonouno
Copy link
Copy Markdown
Contributor

jenkins test this please

@unnonouno
Copy link
Copy Markdown
Contributor

OK, I understand the problem. Usually we need to use fmin for floating point values.

@unnonouno
Copy link
Copy Markdown
Contributor

LGTM

@unnonouno unnonouno merged commit 3a1c4f2 into cupy:master Jul 16, 2018
@unnonouno unnonouno added this to the v5.0.0b3 milestone Jul 16, 2018
@chainer-ci
Copy link
Copy Markdown
Member

Jenkins CI test (for commit 4e33247) succeeded without errors!

@unnonouno
Copy link
Copy Markdown
Contributor

unnonouno commented Jul 16, 2018

Do we need to backport it?

@okuta okuta deleted the fix-min-max branch July 16, 2018 05:33
@okuta
Copy link
Copy Markdown
Member Author

okuta commented Jul 16, 2018

Maybe no.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cat:enhancement Improvements to existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants