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 experimental support for if statements in DALI #4561

Merged
merged 8 commits into from
Jan 19, 2023

Conversation

klecki
Copy link
Contributor

@klecki klecki commented Jan 11, 2023

Signed-off-by: Krzysztof Lecki klecki@nvidia.com

Category: New feature

Description:

Utilize AutoGraph to capture if statements as function with callables wrapping the branches.
Trace the code within both branches, detecting the usage of DataNode.
Each pipeline has a stack of previously checked conditions and entered branches.
Whenever an operator is called within some condition block, it is registered as produced in this scope.
Every time it is used as input to the operator or touched directly via ag__.ld it is looked up in the stack

  • if it was produced on the same level, it is used, otherwise necessary split nodes are inserted.
    Outputs of both branches are detected by AutoGraph. Non-DataNode results are promoted to
    CPU constants (so they represent a batch), and results of both branches go into merge node.

Additional information:

Affected modules and functionalities:

Key points relevant for the review:

Tests:

  • Existing tests apply
  • New tests added
    • Python tests
    • GTests
    • Benchmark
    • Other
  • N/A

Checklist

Documentation

  • Existing documentation applies
  • Documentation updated
    • Docstring
    • Doxygen
    • RST
    • Jupyter
    • Other
  • N/A

DALI team only

Requirements

  • Implements new requirements
  • Affects existing requirements
  • N/A

REQ IDs: N/A

JIRA TASK: N/A

Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Copy link
Contributor Author

klecki commented Jan 11, 2023

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [6990000]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [6990000]: BUILD FAILED

Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Copy link
Contributor Author

klecki commented Jan 12, 2023

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [6996655]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [6996655]: BUILD FAILED

dali/python/nvidia/dali/types.py Outdated Show resolved Hide resolved
f" at {self.stack_depth() -1}:"
f" split({produced_data_node}, predicate={predicate}."))
self._is_registration_allowed = False
true, false = fn._conditional.split(produced_data_node, predicate=predicate)
Copy link
Contributor

Choose a reason for hiding this comment

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

true_node or something like this would be less confusing. At the beginning I thought it was a boolean

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines 71 to 72
def __hash__(self) -> int:
return hash(str(self) + str(self.source))
Copy link
Contributor

Choose a reason for hiding this comment

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

that's unique, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be, but thinking about it, maybe it would be better to use just the repr or string as the keys in the sets. I cannot use the DataNode directly, as it has the __equal__ overloaded to return the arithmetic op.
I could use AutoGraph to replace the comparisons with something else, but I'm not convinced if it's not to many changes and differently working arithmetic ops depending on the mode.

Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Copy link
Contributor Author

klecki commented Jan 12, 2023

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [7000974]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [7000974]: BUILD FAILED

Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Copy link
Contributor Author

klecki commented Jan 13, 2023

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [7008148]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [7008148]: BUILD PASSED

# if pred_node:
# some_op()
# if pred_nested:
# some_other_op()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# some_other_op()
# some_nested_op()

nitpick

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will fix it in a followup

@JanuszL JanuszL mentioned this pull request Jan 17, 2023
Copy link
Contributor

@awolant awolant left a comment

Choose a reason for hiding this comment

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

Looks like it works. Nice!

@klecki klecki merged commit 75431b4 into NVIDIA:main Jan 19, 2023
@klecki klecki deleted the experimental-if-support branch January 19, 2023 13:58
@klecki klecki added the conditional execution Questions related to conditional execution and using `if` statements in DALI label Jan 25, 2023
aderylo pushed a commit to zpp-dali-2022/DALI that referenced this pull request Mar 17, 2023
Utilize AutoGraph to capture if statements as a function that gets 
branches as callables passed as arguments.
Trace the code within both branches, detecting the usage of DataNode.
Each pipeline has a stack of previously checked conditions and entered
branches.
Whenever an operator is called within some condition block, its outputs
(DataNodes) are registered as produced in this scope.
Every time a DataNode is used as input to the operator or touched
directly via `ag__.ld` it is looked up in the stack - if was produced 
on the same level, it is used, otherwise necessary split nodes
are inserted.
Outputs of both branches are detected by AutoGraph.
Non-DataNode results are promoted to CPU constants (so they represent
a batch), and results of both branches go into merge node.

Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conditional execution Questions related to conditional execution and using `if` statements in DALI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants