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

Add comparison operators and bool handling in arithmetic ops #1541

Merged
merged 3 commits into from
Dec 13, 2019

Conversation

klecki
Copy link
Contributor

@klecki klecki commented Dec 4, 2019

Why we need this PR?

It adds comparisons and boolean handling to DALI expressions/arithmetic ops.

What happened in this PR?

  • Explain solution of the problem, new feature added.
    The boolean type was registered for type promotions, support for Constants with booleans and native python bool was added.
    Implementation for all possible comparisons was added, with safe handling of comparisons between singed and unsigned integers.
    Tests were extended and some subset of tests was marked as 'slow' and split into a L1 level test.
  • What was changed, added, removed?
  • What is most important part that reviewers should focus on?
  • Was this PR tested? How?
  • Were docs and examples updated, if necessary?

JIRA TASK: [DALI-1178]

@klecki klecki changed the title Comp ops 00 Add comparison operators and bool handling in arithmetic ops Dec 4, 2019
@klecki
Copy link
Contributor Author

klecki commented Dec 4, 2019

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1018166]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1018166]: BUILD PASSED

auto *result1 = ws.OutputRef<GPUBackend>(1).data<int32_t>();
vector<int32_t> result1_cpu(batch_size * tensor_elements);

MemCopy(result1_cpu.data(), result1, batch_size * tensor_elements * sizeof(int));
Copy link
Contributor

Choose a reason for hiding this comment

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

Add sync

@@ -27,6 +27,29 @@

namespace dali {

template <ArithmeticOp op, typename Result, typename Input0>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just input, in other cases we have left and right. And I don't think we will have 3 arguments arithmetic operators.

Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1029807]: BUILD STARTED

Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1029815]: BUILD STARTED

try:
self.length = len(types)
except TypeError:
types = (types,)
kinds = (kinds,)
self.length = 1
if not disallow_zeros:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: how about a sanity check that disallow_zeros is False when generating bools?

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1029815]: BUILD FAILED

@klecki
Copy link
Contributor Author

klecki commented Dec 12, 2019

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1030422]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1030422]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1030422]: BUILD PASSED

Co-Authored-By: Janusz Lisiecki <39967756+JanuszL@users.noreply.github.com>
@klecki klecki merged commit 2fa3bd1 into NVIDIA:master Dec 13, 2019
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.

None yet

6 participants