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

Numba func notebook #2886

Merged
merged 13 commits into from
May 6, 2021
Merged

Numba func notebook #2886

merged 13 commits into from
May 6, 2021

Conversation

majra20
Copy link
Contributor

@majra20 majra20 commented Apr 21, 2021

Why we need this PR?

  • It adds python notebook example for numba_func

What happened in this PR?

  • What solution was applied:
    NA
  • Affected modules and functionalities:
    NA
  • Key points relevant for the review:
    Clear explanation of numba_func in notebook
  • Validation and testing:
    NA
  • Documentation (including examples):
    NA

JIRA TASK: DALI-1935

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

jantonguirao
jantonguirao reviewed Apr 21, 2021
View reviewed changes
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
klecki
klecki reviewed Apr 22, 2021
View reviewed changes
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated
@@ -0,0 +1,374 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if it won't be easier to pass the types and ndims in the pipeline definition itself. Either way, please explain it why we do it and what the passed values mean. Probably mention that we do not adjust ndim for batch mode.


Reply via ReviewNB

docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
docs/examples/numba_func/Numba_func.ipynb Outdated Show resolved Hide resolved
klecki
klecki reviewed Apr 23, 2021
View reviewed changes
docs/examples/numba_func/Numba_func.ipynb Outdated
"\n",
"This function is compiled. It should be able to work in [nopython](https://numba.pydata.org/numba-doc/latest/user/5minguide.html#what-is-nopython-mode) mode.\n",
"\n",
"#### Define the processing function that fills the output sample base on the input sample"
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
"#### Define the processing function that fills the output sample base on the input sample"
"#### Define the processing function that fills the output sample based on the input sample"

klecki
klecki reviewed Apr 23, 2021
View reviewed changes
docs/examples/numba_func/Numba_func.ipynb Outdated
"id": "impossible-ticket",
"metadata": {},
"source": [
"The run function can have more than one input or output. The function with `n` outputs and `m` inputs should have following definition:\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe signature instead of definition?

klecki
klecki reviewed Apr 23, 2021
View reviewed changes
docs/examples/numba_func/Numba_func.ipynb Outdated
"metadata": {},
"source": [
"### Multiple inputs and outputs\n",
"\"The numba function operator can take up to 6 inputs and produce up to 6 outputs. As an example, let us write a pipeline that takes an image and splits the RGB channels into 3 separate outputs\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

You have additional " in text here.

@klecki
Copy link
Contributor

klecki commented Apr 23, 2021

Please mention the ndims and types that we need to specify. Otherwise ok.

@majra20
Copy link
Contributor Author

majra20 commented Apr 27, 2021

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2309134]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2309134]: BUILD PASSED

@klecki
Copy link
Contributor

klecki commented Apr 27, 2021

Sorry I didn't spot it earlier, but can you move this file to docs/examples/custom_operations
and adjust the index.rst there so it is included in the rendered docs here: https://docs.nvidia.com/deeplearning/dali/user-guide/docs/examples/custom_operations/index.html

@majra20
Copy link
Contributor Author

majra20 commented Apr 28, 2021

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2312814]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2312814]: BUILD PASSED

mzient
mzient reviewed Apr 29, 2021
View reviewed changes
docs/examples/custom_operations/numba_function.ipynb
@@ -0,0 +1,468 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

presents -> shows (we present that the tutorial)

user function -> user-defined function

foreign C code -> external C code

utils -> utilities (no jargon)


Reply via ReviewNB

Copy link
Contributor

Choose a reason for hiding this comment

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

custom operations written as a Numba JIT-compiled functions.

docs/examples/custom_operations/numba_function.ipynb
@@ -0,0 +1,468 @@
{
Copy link
Contributor

@mzient mzient Apr 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

pipeline which rotates images by 90 degrees

Since the output shape will be different than the input shape

In the setup function, we define the output

shapes of the outputs are the same as the in the inputs.


Reply via ReviewNB

docs/examples/custom_operations/numba_function.ipynb
@@ -0,0 +1,468 @@
{
Copy link
Contributor

@mzient mzient Apr 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

and a list of input shapes, which contains the input shape

The first dimension is the index of the output/input

It's not a multidimensional array, but a nested list. I'd rephrase it:

The outer index applied to outs/ins is the index of the respective output/input; the inner index indicates the sample in the batch.


Reply via ReviewNB

Rafal Maj added 11 commits May 5, 2021 09:55
Python notebook
1795db2 
Signed-off-by: Rafal Maj <rmaj@nvidia.com>
update execution count
16f6219 
Signed-off-by: Rafal Maj <rmaj@nvidia.com>
code review suggestions
f414ca6 
Signed-off-by: Rafal Maj <rmaj@nvidia.com>
Code review changes
fa77fb8 
Signed-off-by: Rafal Maj <rmaj@nvidia.com>
Code review changes
ee16afa 
Signed-off-by: Rafal Maj <rmaj@nvidia.com>
Code review changes
806eb9f 
Signed-off-by: Rafal Maj <rmaj@nvidia.com>
change numba_function name
c493534 
Signed-off-by: Rafal Maj <rmaj@nvidia.com>
Rebase
e6a615a 
Signed-off-by: Rafal Maj <rmaj@nvidia.com>
Rename numba_func
e9151db 
Signed-off-by: Rafal Maj <rmaj@nvidia.com>
moving notebook
bc46635 
Signed-off-by: Rafal Maj <rmaj@nvidia.com>
code review changes
a797cfb 
Signed-off-by: Rafal Maj <rmaj@nvidia.com>
@majra20
Copy link
Contributor Author

majra20 commented May 5, 2021

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2336829]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2336829]: BUILD FAILED

@majra20
Copy link
Contributor Author

majra20 commented May 5, 2021

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2336909]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2336909]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2336988]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2336989]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2336988]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2336989]: BUILD FAILED

Rafal Maj added 2 commits May 5, 2021 12:41
style fix
b9f8b83 
Signed-off-by: Rafal Maj <rmaj@nvidia.com>
style fix
ecbda94 
Signed-off-by: Rafal Maj <rmaj@nvidia.com>
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2337008]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2337011]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2337008]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2337008]: BUILD PASSED

@majra20 majra20 merged commit bed7853 into NVIDIA:master May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jantonguirao jantonguirao jantonguirao left review comments

@klecki klecki klecki approved these changes

@mzient mzient mzient approved these changes

Assignees

@klecki klecki

@mzient mzient

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@majra20 @klecki @dali-automaton @jantonguirao @mzient