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

Make Workspace::Input return const reference #3452

Merged
merged 7 commits into from
Oct 27, 2021
Merged

Make Workspace::Input return const reference #3452

merged 7 commits into from
Oct 27, 2021

Conversation

klecki
Copy link
Contributor

@klecki klecki commented Oct 26, 2021 •
edited
Loading

Description

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (Redesign of existing code that doesn't affect functionality)
  • Other (e.g. Documentation, Tests, Configuration)

What happened in this PR

Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally.

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Additional information

  • Affected modules and functionalities:
  • Key points relevant for the review:

Checklist

Tests

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

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

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

mzient
mzient reviewed Oct 26, 2021
View reviewed changes
dali/operators/decoder/audio/audio_decoder_op.cc Outdated
@@ -88,7 +88,7 @@ AudioDecoderCpu::SetupImpl(std::vector<OutputDesc> &output_desc, const workspace

for (int i = 0; i < batch_size; i++) {
auto &meta = sample_meta_[i] =
decoders_[i]->Open({reinterpret_cast<const char *>(input[i].raw_mutable_data()),
decoders_[i]->Open({reinterpret_cast<const char *>(input[i].raw_data()),
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
decoders_[i]->Open({reinterpret_cast<const char *>(input[i].raw_data()),
decoders_[i]->Open({static_cast<const char *>(input[i].raw_data()),

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

@klecki klecki force-pushed the constness-problem-input-ref branch from 30a1db5 to a26d1b3 Compare October 26, 2021 15:33
@klecki
Cast fix
2c96bb6 
Signed-off-by: Krzysztof Lecki <[email protected]>
@awolant awolant self-assigned this Oct 26, 2021
@klecki
Move const cast by one level
cf792b0 
Signed-off-by: Krzysztof Lecki <[email protected]>
@klecki
comment
6983c69 
Signed-off-by: Krzysztof Lecki <[email protected]>
@klecki
Copy link
Contributor Author

klecki commented Oct 26, 2021

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [3268958]: BUILD STARTED

@klecki
Fixup
4d69854 
Signed-off-by: Krzysztof Lecki <[email protected]>
@klecki
Copy link
Contributor Author

klecki commented Oct 26, 2021

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [3268958]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [3269559]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [3269559]: BUILD PASSED

@klecki
Add docs and UnsafeMutableInput
77e3c28 
Signed-off-by: Krzysztof Lecki <[email protected]>
mzient
mzient reviewed Oct 27, 2021
View reviewed changes
dali/pipeline/data/dltensor.h Outdated
@@ -57,12 +57,13 @@ DLMTensorPtr GetDLTensorView(Tensor<Backend> &tensor) {
}

template <typename Backend>
std::vector<DLMTensorPtr> GetDLTensorListView(TensorList<Backend> &tensor_list) {
std::vector<DLMTensorPtr> GetDLTensorListView(const TensorList<Backend> &tensor_list) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we should pass a non-const TL here and adjust the call site to use UnsafeMutableInput.

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

@klecki
Allow DLPack mutable access
6084550 
Signed-off-by: Krzysztof Lecki <[email protected]>
@klecki
Copy link
Contributor Author

klecki commented Oct 27, 2021

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [3275202]: BUILD STARTED

@NVIDIA NVIDIA deleted a comment from dali-automaton Oct 27, 2021
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [3275202]: BUILD PASSED

@klecki klecki merged commit d276ea2 into NVIDIA:main Oct 27, 2021
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
3ab398e 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
e744ec4 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
e5cd9cb 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
8996f04 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
b688fc4 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
722ea5f 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
e3c5746 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
c6ccf70 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
623e0f3 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
566f3c6 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
d3aabd1 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
e2ec80e 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
ca59f30 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Feb 21, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
ba3800a 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
@JanuszL JanuszL mentioned this pull request Mar 30, 2022
Closed
cyyever pushed a commit to cyyever/DALI that referenced this pull request May 13, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
e274cf1 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jun 7, 2022
@klecki @cyyever
Make Workspace::Input return const reference (NVIDIA#3452)
7a581c8 
Input (previously InputRef) was returning
mutable reference to data by mistake.

Fix the constness of the returned reference,
adjust misuses of the mutable accessors.

Introduce UnsafeMutableInput for access
in the executor and related utilities,
where inputs needs to be adjusted additionally. 

Add docs.

Adjust ShareData to accept const ref,
which is needed after this change.

Use UnsafeMutableInput for DLPack function
implementation.

Signed-off-by: Krzysztof Lecki <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mzient mzient mzient approved these changes

@awolant awolant awolant approved these changes

Assignees

@mzient mzient

@awolant awolant

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@klecki @dali-automaton @mzient @awolant