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.

Sign up for GitHub

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

Jump to bottom

Add origin stack trace capture for DALI operators #5302

Merged
merged 34 commits into from
Mar 4, 2024
Merged

Add origin stack trace capture for DALI operators #5302

merged 34 commits into from
Mar 4, 2024

Conversation

klecki
Copy link
Contributor

@klecki klecki commented Feb 1, 2024 •
edited
Loading

Category: New feature

Description:

Add capture of origin stack trace for DALI operators.
The extract_stack collects all frames from the first frame of pipeline definition to the frame with operator invocation (the outermost user API).
In regular mode no further processing is needed.
For code that was transformed by AutoGraph, the contents of captured frames are remapped back to the user code, filtered out of the _autograph and _conditionals modules (that contain internal DALI implementation).
Due to how autograph introduces additional frames (for example by implementing if statements with additional function calls), we remove repeated occurrences of the same function, keeping only the last one. AutoGraph's entry point to a function call is used to detect such regions.

Stack traces are collected in OperatorInstance, the outermost layer of the API adds an argument denoting the current stack level, so we can compute how many frames we need to skip related to DALI internals when capturing the stack.

The collected stack is added as hidden arguments to OpSpec. This is backward-compatible for serialized pipelines, as well as allows for disabling and enabling the feature.

As a default the feature is disabled and only available via hidden API.

C++ test operators are extended to work as a loadable plugin based on dummy operator test.
It allows to implement tests for the new feature without extending regular DALI operators.

Follow-up to this PR will utilize this information for providing better error messages - pointing to the origin of error in DALI pipeline definition.

Additional information:

Affected modules and functionalities:

New functionality, extensions for Python API and new hidden arguments in all operators.

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

github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 1, 2024
View reviewed changes
dali/python/nvidia/dali/_autograph/utils/tf_stack.py Dismissed Show dismissed Hide dismissed
dali/test/python/operator_1/test_operator_origin_trace.py Fixed Show fixed Hide fixed
dali/test/python/operator_1/test_operator_origin_trace.py Fixed Show fixed Hide fixed
dali/python/nvidia/dali/_autograph/impl/api.py
raise
# We no longer need CurrentModuleFilter here, as we filter whole autograph
import nvidia.dali._conditionals as dc
import nvidia.dali._autograph as ag

Check notice

Code scanning / CodeQL

Module is imported with 'import' and 'import from' Note

Module 'nvidia.dali._autograph' is imported with both 'import' and 'import from'.
Module '_autograph' is imported with both 'import' and 'import from'.
@stiepan stiepan self-assigned this Feb 2, 2024
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 15, 2024
View reviewed changes
dali/test/python/operator_1/test_operator_origin_trace.py Fixed Show fixed Hide fixed
dali/test/python/operator_1/test_operator_origin_trace.py Fixed Show fixed Hide fixed
dali/test/python/operator_1/test_operator_origin_trace.py

def pipe():
if 0:
x = origin_trace()

Check warning

Code scanning / CodeQL

Unreachable code Warning test

This statement is unreachable.
@klecki
Remove the TF implementation files
b6e68a8 
Some parts are rewritten in DALI in following commits

Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Reimplement parts of tf_stack with pure Python. Enable stack remapping
fbde70b 
The solution to stack-trace manipulation in TF was optimized by moving
parts of the processing to C++. As we have limited needs for producing
traces, it is sufficient to replace them with regulart Python
constructs. This steps reenables the construction of StackTraceMapper
and Filter in the AG-converted code.

It will enable next steps:
* we will be capturing origin stack trace for every operator defintion
* in conditional mode we will remap it back to the user code from the
  AG-produced one.

Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Introduce draft of the extract_stack reworked for DALI purposes
f1f256c 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Try to propagate original source_code_line in the source maps
4ea7c48 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Add extraction code
72db778 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Add ability for using plugin operators for python test
26b5360 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Some linting
435f928 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
WIP
84e110a 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Filtering seems to work
62ac62b 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Something is close to working
049404f 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Clean up the test
d8d1118 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
TMP work
66a239b 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Maybe we just don't filter _conditionals.py
0d3ecd5 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
it seems to work a bit better
f7609f0 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Almost API handling
ad86e14 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
It works :)
c653db1 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Comments
1d6f559 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
CLeanup
1d8131d 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Cleanup
65ee7af 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
CLeanip
05ae4de
@klecki
Fix and improvement
c2dee1f 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Review
176f1a7 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Copy link
Contributor Author

klecki commented Feb 28, 2024

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [13153535]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [13153535]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [13153535]: BUILD PASSED

klecki added a commit to klecki/DALI that referenced this pull request Feb 29, 2024
@klecki
Port more things from NVIDIA#5302
9509ddc 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
klecki added a commit to klecki/DALI that referenced this pull request Feb 29, 2024
@klecki
Port more things from NVIDIA#5302
2ceb821 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Naming and comments
76a96e0 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
It should work with trailing "/"
b909d6f 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@klecki
Copy link
Contributor Author

klecki commented Mar 4, 2024

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [13262456]: BUILD STARTED

klecki added a commit to klecki/DALI that referenced this pull request Mar 4, 2024
@klecki
Port more things from NVIDIA#5302
5f4282d 
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [13262456]: BUILD PASSED

@klecki klecki merged commit 83e61e5 into NVIDIA:main Mar 4, 2024
6 of 7 checks passed
@klecki klecki deleted the origin-stack-trace branch March 4, 2024 20:47
@stiepan stiepan mentioned this pull request Mar 7, 2024
Merged
18 tasks
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stiepan stiepan stiepan approved these changes

@mzient mzient mzient approved these changes

@szkarpinski szkarpinski szkarpinski approved these changes

Assignees

@stiepan stiepan

@mzient mzient

@szkarpinski szkarpinski

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@klecki @dali-automaton @stiepan @mzient @szkarpinski @banasraf