sliding-window-inferer #765
Conversation
Signed-off-by: Liqun Fu <[email protected]>
Signed-off-by: Liqun Fu <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
liqunfu
changed the title
Signed-off-by: Liqun Fu <[email protected]>
Signed-off-by: Liqun Fu <[email protected]>
|
|
||
| import numpy as np | ||
|
|
||
| from onnxscript import script |
Check notice
Code scanning / CodeQL
Unused import
|
|
||
| from onnxscript import script | ||
| from onnxscript.onnx_opset import opset15 as op | ||
| from onnxscript.onnx_types import FLOAT, INT64 |
Check notice
Code scanning / CodeQL
Unused import
| from onnxscript import script | ||
| from onnxscript.onnx_opset import opset15 as op | ||
| from onnxscript.onnx_types import FLOAT, INT64 | ||
| from onnxscript.tests.common import onnx_script_test_case, testutils |
Check notice
Code scanning / CodeQL
Unused import
| # if sw_batch_size > 1: | ||
| # for idx in op.Range(slice_g + 1, op.Min(slice_g + sw_batch_size, num_win)): | ||
| # win_data = op.Concat(win_data, op.Slice(inputs, slices[idx, :, 0], slices[idx, :, 1], spatial_axes)) |
Check notice
Code scanning / CodeQL
Commented-out code
| from onnxscript.tests.models.pnp import roi_indices_3d, aggrregate_predictor_output, sliding_window_inference, predict_mock, predict_mock_2 | ||
|
|
||
| class PnpOpTest(onnx_script_test_case.OnnxScriptTestCase): | ||
| def test_roi_indices_3d(delf): |
Check notice
Code scanning / CodeQL
First parameter of a method is not named 'self'
Signed-off-by: Liqun Fu <[email protected]>
|
|
||
| save_model = False | ||
| if save_model: | ||
| model = sliding_window_inference.function_ir.to_model_proto(producer_name="monai") |
Check warning
Code scanning / CodeQL
Unreachable code
| from onnx import TensorProto | ||
| from onnx.helper import make_tensor | ||
|
|
||
| from onnxscript import script, graph |
Check notice
Code scanning / CodeQL
Unused import
| from typing import ( | ||
| Any, | ||
| Callable, | ||
| Mapping, | ||
| ) |
Check notice
Code scanning / CodeQL
Unused import
| from onnxscript.onnx_types import ( | ||
| BFLOAT16, | ||
| BOOL, | ||
| COMPLEX64, | ||
| COMPLEX128, | ||
| DOUBLE, | ||
| FLOAT, | ||
| FLOAT16, | ||
| INT8, | ||
| INT16, | ||
| INT32, | ||
| INT64, | ||
| STRING, | ||
| UINT8, | ||
| UINT16, | ||
| UINT32, | ||
| UINT64, | ||
| ) |
Check notice
Code scanning / CodeQL
Unused import
| UINT32, | ||
| UINT64, | ||
| ) | ||
| from onnxscript.values import Op, Opset |
Check notice
Code scanning / CodeQL
Unused import
| UINT64, | ||
| ) | ||
| from onnxscript.values import Op, Opset | ||
| from typing import Callable, Optional, Sequence, Tuple, Union |
Check notice
Code scanning / CodeQL
Unused import
| from onnxscript.values import Op, Opset | ||
| from typing import Callable, Optional, Sequence, Tuple, Union | ||
|
|
||
| import numpy as np |
Check notice
Code scanning / CodeQL
Module is imported more than once
|
|
||
| import numpy as np | ||
| import onnxruntime as rt | ||
| import onnx |
Check notice
Code scanning / CodeQL
Module is imported more than once
| import onnxruntime as rt | ||
| import onnx | ||
|
|
||
| class Opset18Ext(Opset18): |
There was a problem hiding this comment.
Why do you need an opset for this? Can we just define OpaqueOp as a normal python function and use it?
There was a problem hiding this comment.
I need a custom operator so that it can be programmatically edited to replace the op with the core model graph. I should not has used onnx domain though.
onnxscript/tests/models/pnp.py
Outdated
| """ | ||
|
|
||
| inputs_shape = op.Shape(inputs) | ||
| inputs_spatial_shape = op.Slice(inputs_shape, op.Constant(value_ints=[2]), op.Constant(value_ints=[5]), op.Constant(value_ints=[0])) |
There was a problem hiding this comment.
nit: may be this will be more readable if we move it after the line below, and rewrite it to:
inputs_spatial_shape = op.Concat(D, H, W, axis=0)Alternatively, we can also say:
inputs_spatial_shape = op.Shape(inputs, from=2)
onnxscript/tests/models/pnp.py
Outdated
| return indices, roi_shape | ||
|
|
||
| @script() | ||
| def aggrregate_predictor_output( |
There was a problem hiding this comment.
nit: spelling "aggregate"
onnxscript/tests/models/pnp.py
Outdated
| one = op.Constant(value_int=1) | ||
| one_ = op.Constant(value_ints=[1]) | ||
| roi_D, roi_H, roi_W, _ = op.Split(op.Shape(roi_indices), num_outputs=4) | ||
| shape_1_seg_C_roi_D_H_W_1 = op.Concat(one_, seg_C, roi_D, roi_H, roi_W, one_, axis=0) |
There was a problem hiding this comment.
nit: you can directly use op.Shape(roi_indices) instead of roi_D, roi_H, roi_W ... no need to split it and concat it back.
onnxscript/tests/models/pnp.py
Outdated
| one_ = op.Constant(value_ints=[1]) | ||
| roi_D, roi_H, roi_W, _ = op.Split(op.Shape(roi_indices), num_outputs=4) | ||
| shape_1_seg_C_roi_D_H_W_1 = op.Concat(one_, seg_C, roi_D, roi_H, roi_W, one_, axis=0) | ||
| zeros_1_seg_C_D_H_W_1 = op.Cast(op.ConstantOfShape(shape_1_seg_C_roi_D_H_W_1), to=INT64.dtype) # seg_C, roi_D, roi_H, roi_W, 1 |
There was a problem hiding this comment.
nit: you can directly specify the value/type as attribute of ConstantOfShape, instead of casting by adding value=onnx.helper.make_tensor(...)
| grid_w = grid_w_0 + zeros_D_H_W | ||
|
|
||
| indices_seq = op.SequenceConstruct(grid_d, grid_h, grid_w) | ||
| indices = op.ConcatFromSequence(indices_seq, axis=-1, new_axis=1) # [D, H, W, 3] |
There was a problem hiding this comment.
I guess you need this because Concat doesn't support the new_axis attributes? I wonder if we should extend Concat to support it.
We could explicitly unsqueeze grid_d etc. to add the extra dimension at end and then use Concat. It is a bit more cumbersome, but onnxruntime doesn't handle sequence op that well (it may create extra copies of the tensor). Anyway, may not be worth bothering about for now.
…n a loop and passed Signed-off-by: Liqun Fu <[email protected]>
| for i in range(start[0], stop[0]): | ||
| for j in range(start[1], stop[1]): | ||
| for k in range(start[2], stop[2]): | ||
| aggrregated_pred_expected[i, j, k] += pred[i - start[0], j - start[1], k - start[2]] |
Check failure
Code scanning / CodeQL
Modification of parameter with default
| for i in range(start[0], stop[0]): | ||
| for j in range(start[1], stop[1]): | ||
| for k in range(start[2], stop[2]): | ||
| aggrregated_pred_expected[i, j, k] += pred[i - start[0], j - start[1], k - start[2]] |
Check failure
Code scanning / CodeQL
Modification of parameter with default
| for j in range(start[1], stop[1]): | ||
| for k in range(start[2], stop[2]): | ||
| aggrregated_pred_expected[i, j, k] += pred[i - start[0], j - start[1], k - start[2]] | ||
| aggrregated_count_expected[i, j, k] += 1 |
Check failure
Code scanning / CodeQL
Modification of parameter with default
| for j in range(start[1], stop[1]): | ||
| for k in range(start[2], stop[2]): | ||
| aggrregated_pred_expected[i, j, k] += pred[i - start[0], j - start[1], k - start[2]] | ||
| aggrregated_count_expected[i, j, k] += 1 |
Check failure
Code scanning / CodeQL
Modification of parameter with default
Codecov Report
@@ Coverage Diff @@
## main #765 +/- ##
==========================================
- Coverage 76.87% 76.45% -0.43%
==========================================
Files 112 114 +2
Lines 13736 14011 +275
Branches 1407 1430 +23
==========================================
+ Hits 10560 10712 +152
- Misses 2824 2935 +111
- Partials 352 364 +12
|
Test Results 18 files ± 0 18 suites ±0 2h 19m 27s ⏱️ + 1h 12m 27s For more details on these failures, see this check. Results for commit 9d418ba. ± Comparison against base commit 176141e. This pull request removes 1 and adds 4 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Signed-off-by: Liqun Fu <[email protected]>
…//github.com/microsoft/onnx-script into liqun/onnx_script_test_case_multuple_outputs
| from onnxscript.tests.models.pnp import ( | ||
| roi_indices_3d, | ||
| aggregate_predictor_output, | ||
| sliding_window_inference, | ||
| predict_mock, | ||
| predict_mock_2, | ||
| Opset18Ext, | ||
| get_scan_interval_script, | ||
| ) |
Check notice
Code scanning / CodeQL
Unused import
| inputs_shape = op.Shape(inputs) | ||
| inputs_spatial_shape = op.Shape(inputs, start=2) | ||
| N, _, D, H, W = op.Split(inputs_shape, num_outputs=5) | ||
| roi_D, roi_H, roi_W = op.Split(roi_size, num_outputs=3) |
Check notice
Code scanning / CodeQL
Unused local variable
| inputs_shape = op.Shape(inputs) | ||
| inputs_spatial_shape = op.Shape(inputs, start=2) | ||
| N, _, D, H, W = op.Split(inputs_shape, num_outputs=5) | ||
| roi_D, roi_H, roi_W = op.Split(roi_size, num_outputs=3) |
Check notice
Code scanning / CodeQL
Unused local variable
| inputs_shape = op.Shape(inputs) | ||
| inputs_spatial_shape = op.Shape(inputs, start=2) | ||
| N, _, D, H, W = op.Split(inputs_shape, num_outputs=5) | ||
| roi_D, roi_H, roi_W = op.Split(roi_size, num_outputs=3) |
Check notice
Code scanning / CodeQL
Unused local variable
| use 1 instead to make sure sliding window works. | ||
| """ | ||
| zero = op.Constant(value_int=0) | ||
| scan_interval = zero |
Check warning
Code scanning / CodeQL
Variable defined multiple times
No description provided.