-
Notifications
You must be signed in to change notification settings - Fork 228
Add pad_left option to PadRagged for left-side padding support #1338
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
Conversation
Can you also add a test? Please see k2/python/tests folder. There are many test files. |
OK |
fix redeclaration fix pybind fix pybind back
I have added corresponding Running tests...
Test project /home/v-yifyang/k2/build_release
Start 1: Test.Cuda.cu_algorithms_test
1/111 Test #1: Test.Cuda.cu_algorithms_test ................. Passed 1.04 sec
Start 2: Test.Cuda.cu_array_of_ragged_test
2/111 Test #2: Test.Cuda.cu_array_of_ragged_test ............ Passed 1.00 sec
Start 3: Test.Cuda.cu_array_ops_test
3/111 Test #3: Test.Cuda.cu_array_ops_test .................. Passed 1.63 sec
Start 4: Test.Cuda.cu_array_test
4/111 Test #4: Test.Cuda.cu_array_test ...................... Passed 1.00 sec
Start 5: Test.Cuda.cu_connect_test
5/111 Test #5: Test.Cuda.cu_connect_test .................... Passed 1.00 sec
Start 6: Test.Cuda.cu_dtype_test
6/111 Test #6: Test.Cuda.cu_dtype_test ...................... Passed 0.40 sec
Start 7: Test.Cuda.cu_fsa_algo_test
7/111 Test #7: Test.Cuda.cu_fsa_algo_test ................... Passed 11.35 sec
Start 8: Test.Cuda.cu_fsa_test
8/111 Test #8: Test.Cuda.cu_fsa_test ........................ Passed 1.00 sec
Start 9: Test.Cuda.cu_fsa_utils_test
9/111 Test #9: Test.Cuda.cu_fsa_utils_test .................. Passed 1.65 sec
Start 10: Test.Cuda.cu_hash_test
10/111 Test #10: Test.Cuda.cu_hash_test .......................***Failed 1.13 sec
Start 11: Test.Cuda.cu_host_shim_test
11/111 Test #11: Test.Cuda.cu_host_shim_test .................. Passed 0.39 sec
Start 12: Test.Cuda.cu_intersect_test
12/111 Test #12: Test.Cuda.cu_intersect_test .................. Passed 1.18 sec
Start 13: Test.Cuda.cu_log_test
13/111 Test #13: Test.Cuda.cu_log_test ........................ Passed 0.98 sec
Start 14: Test.Cuda.cu_macros_test
14/111 Test #14: Test.Cuda.cu_macros_test ..................... Passed 0.99 sec
Start 15: Test.Cuda.cu_math_test
15/111 Test #15: Test.Cuda.cu_math_test ....................... Passed 0.40 sec
Start 16: Test.Cuda.cu_nbest_test
16/111 Test #16: Test.Cuda.cu_nbest_test ...................... Passed 0.39 sec
Start 17: Test.Cuda.cu_nvtx_test
17/111 Test #17: Test.Cuda.cu_nvtx_test ....................... Passed 4.39 sec
Start 18: Test.Cuda.cu_pinned_context_test
18/111 Test #18: Test.Cuda.cu_pinned_context_test ............. Passed 14.94 sec
Start 19: Test.Cuda.cu_ragged_shape_test
19/111 Test #19: Test.Cuda.cu_ragged_shape_test ............... Passed 1.00 sec
Start 20: Test.Cuda.cu_ragged_test
20/111 Test #20: Test.Cuda.cu_ragged_test ..................... Passed 1.73 sec
Start 21: Test.Cuda.cu_ragged_utils_test
21/111 Test #21: Test.Cuda.cu_ragged_utils_test ............... Passed 0.99 sec
Start 22: Test.Cuda.cu_rand_test
22/111 Test #22: Test.Cuda.cu_rand_test ....................... Passed 1.02 sec
Start 23: Test.Cuda.cu_reverse_test
23/111 Test #23: Test.Cuda.cu_reverse_test .................... Passed 0.99 sec
Start 24: Test.Cuda.cu_rm_epsilon_test
24/111 Test #24: Test.Cuda.cu_rm_epsilon_test ................. Passed 1.50 sec
Start 25: Test.Cuda.cu_rnnt_decode_test
25/111 Test #25: Test.Cuda.cu_rnnt_decode_test ................ Passed 1.01 sec
Start 26: Test.Cuda.cu_tensor_ops_test
26/111 Test #26: Test.Cuda.cu_tensor_ops_test ................. Passed 1.05 sec
Start 27: Test.Cuda.cu_tensor_test
27/111 Test #27: Test.Cuda.cu_tensor_test ..................... Passed 0.39 sec
Start 28: Test.Cuda.cu_thread_pool_test
28/111 Test #28: Test.Cuda.cu_thread_pool_test ................ Passed 0.41 sec
Start 29: Test.Cuda.cu_top_sort_test
29/111 Test #29: Test.Cuda.cu_top_sort_test ................... Passed 1.32 sec
Start 30: Test.Cuda.cu_utils_test
30/111 Test #30: Test.Cuda.cu_utils_test ...................... Passed 0.99 sec
Start 31: Test.arcsort_test
31/111 Test #31: Test.arcsort_test ............................ Passed 0.00 sec
Start 32: Test.array_test
32/111 Test #32: Test.array_test .............................. Passed 0.00 sec
Start 33: Test.aux_labels_test
33/111 Test #33: Test.aux_labels_test ......................... Passed 0.00 sec
Start 34: Test.connect_test
34/111 Test #34: Test.connect_test ............................ Passed 0.00 sec
Start 35: Test.determinize_test
35/111 Test #35: Test.determinize_test ........................ Passed 0.01 sec
Start 36: Test.fsa_equivalent_test
36/111 Test #36: Test.fsa_equivalent_test ..................... Passed 0.01 sec
Start 37: Test.fsa_renderer_test
37/111 Test #37: Test.fsa_renderer_test ....................... Passed 0.00 sec
Start 38: Test.fsa_test
38/111 Test #38: Test.fsa_test ................................ Passed 0.00 sec
Start 39: Test.fsa_util_test
39/111 Test #39: Test.fsa_util_test ........................... Passed 0.00 sec
Start 40: Test.intersect_test
40/111 Test #40: Test.intersect_test .......................... Passed 0.00 sec
Start 41: Test.properties_test
41/111 Test #41: Test.properties_test ......................... Passed 0.00 sec
Start 42: Test.rmepsilon_test
42/111 Test #42: Test.rmepsilon_test .......................... Passed 0.00 sec
Start 43: Test.topsort_test
43/111 Test #43: Test.topsort_test ............................ Passed 0.00 sec
Start 44: Test.weights_test
44/111 Test #44: Test.weights_test ............................ Passed 0.00 sec
Start 45: add_epsilon_self_loops_test_py
45/111 Test #45: add_epsilon_self_loops_test_py ............... Passed 1.86 sec
Start 46: arc_sort_test_py
46/111 Test #46: arc_sort_test_py ............................. Passed 1.83 sec
Start 47: cat_test_py
47/111 Test #47: cat_test_py .................................. Passed 1.81 sec
Start 48: compose_arc_maps_test_py
48/111 Test #48: compose_arc_maps_test_py ..................... Passed 1.80 sec
Start 49: closure_test_py
49/111 Test #49: closure_test_py .............................. Passed 1.88 sec
Start 50: compose_test_py
50/111 Test #50: compose_test_py .............................. Passed 1.42 sec
Start 51: connect_test_py
51/111 Test #51: connect_test_py .............................. Passed 1.87 sec
Start 52: create_sparse_test_py
52/111 Test #52: create_sparse_test_py ........................ Passed 1.81 sec
Start 53: ctc_graph_test_py
53/111 Test #53: ctc_graph_test_py ............................ Passed 1.80 sec
Start 54: ctc_loss_test_py
54/111 Test #54: ctc_loss_test_py ............................. Passed 2.02 sec
Start 55: ctc_topo_test_py
55/111 Test #55: ctc_topo_test_py ............................. Passed 1.17 sec
Start 56: dense_fsa_vec_test_py
56/111 Test #56: dense_fsa_vec_test_py ........................ Passed 1.94 sec
Start 57: determinize_test_py
57/111 Test #57: determinize_test_py .......................... Passed 1.15 sec
Start 58: expand_ragged_attributes_test_py
58/111 Test #58: expand_ragged_attributes_test_py ............. Passed 1.91 sec
Start 59: fsa_from_unary_function_ragged_test_py
59/111 Test #59: fsa_from_unary_function_ragged_test_py ....... Passed 1.92 sec
Start 60: fsa_from_unary_function_tensor_test_py
60/111 Test #60: fsa_from_unary_function_tensor_test_py ....... Passed 1.92 sec
Start 61: fsa_test_py
61/111 Test #61: fsa_test_py .................................. Passed 2.60 sec
Start 62: get_arc_post_test_py
62/111 Test #62: get_arc_post_test_py ......................... Passed 2.13 sec
Start 63: get_backward_scores_test_py
63/111 Test #63: get_backward_scores_test_py .................. Passed 2.04 sec
Start 64: get_forward_scores_test_py
64/111 Test #64: get_forward_scores_test_py ................... Passed 2.02 sec
Start 65: get_best_matching_stats_test_py
65/111 Test #65: get_best_matching_stats_test_py .............. Passed 1.21 sec
Start 66: get_tot_scores_test_py
66/111 Test #66: get_tot_scores_test_py ....................... Passed 1.98 sec
Start 67: index_add_test_py
67/111 Test #67: index_add_test_py ............................ Passed 2.41 sec
Start 68: index_and_sum_test_py
68/111 Test #68: index_and_sum_test_py ........................ Passed 1.91 sec
Start 69: index_select_test_py
69/111 Test #69: index_select_test_py ......................... Passed 2.57 sec
Start 70: index_test_py
70/111 Test #70: index_test_py ................................ Passed 1.99 sec
Start 71: intersect_dense_pruned_test_py
71/111 Test #71: intersect_dense_pruned_test_py ............... Passed 2.07 sec
Start 72: intersect_dense_test_py
72/111 Test #72: intersect_dense_test_py ...................... Passed 2.06 sec
Start 73: intersect_device_test_py
73/111 Test #73: intersect_device_test_py ..................... Passed 2.01 sec
Start 74: intersect_test_py
74/111 Test #74: intersect_test_py ............................ Passed 1.95 sec
Start 75: invert_test_py
75/111 Test #75: invert_test_py ............................... Passed 1.90 sec
Start 76: levenshtein_alignment_test_py
76/111 Test #76: levenshtein_alignment_test_py ................ Passed 1.92 sec
Start 77: levenshtein_graph_test_py
77/111 Test #77: levenshtein_graph_test_py .................... Passed 1.88 sec
Start 78: linear_fsa_test_py
78/111 Test #78: linear_fsa_test_py ........................... Passed 1.87 sec
Start 79: linear_fsa_with_self_loops_test_py
79/111 Test #79: linear_fsa_with_self_loops_test_py ........... Passed 1.87 sec
Start 80: linear_fst_test_py
80/111 Test #80: linear_fst_test_py ........................... Passed 1.19 sec
Start 81: linear_fst_with_self_loops_test_py
81/111 Test #81: linear_fst_with_self_loops_test_py ........... Passed 1.96 sec
Start 82: multi_gpu_test_py
82/111 Test #82: multi_gpu_test_py ............................ Passed 1.51 sec
Start 83: mutual_information_test_py
83/111 Test #83: mutual_information_test_py ................... Passed 18.93 sec
Start 84: mwer_test_py
84/111 Test #84: mwer_test_py ................................. Passed 1.55 sec
Start 85: nbest_test_py
85/111 Test #85: nbest_test_py ................................ Passed 1.57 sec
Start 86: numerical_gradient_check_test_py
86/111 Test #86: numerical_gradient_check_test_py ............. Passed 4.25 sec
Start 87: online_dense_intersecter_test_py
87/111 Test #87: online_dense_intersecter_test_py ............. Passed 1.96 sec
Start 88: pruned_ranges_to_lattice_test_py
88/111 Test #88: pruned_ranges_to_lattice_test_py ............. Passed 1.51 sec
Start 89: ragged_ops_test_py
89/111 Test #89: ragged_ops_test_py ........................... Passed 2.06 sec
Start 90: ragged_shape_test_py
90/111 Test #90: ragged_shape_test_py ......................... Passed 1.91 sec
Start 91: ragged_tensor_test_py
91/111 Test #91: ragged_tensor_test_py ........................ Passed 2.03 sec
Start 92: ragged_test_py
92/111 Test #92: ragged_test_py ............................... Passed 1.91 sec
Start 93: random_paths_test_py
93/111 Test #93: random_paths_test_py ......................... Passed 1.90 sec
Start 94: remove_epsilon_self_loops_test_py
94/111 Test #94: remove_epsilon_self_loops_test_py ............ Passed 1.95 sec
Start 95: remove_epsilon_test_py
95/111 Test #95: remove_epsilon_test_py ....................... Passed 1.97 sec
Start 96: reverse_test_py
96/111 Test #96: reverse_test_py .............................. Passed 1.87 sec
Start 97: rnnt_decode_test_py
97/111 Test #97: rnnt_decode_test_py .......................... Passed 1.85 sec
Start 98: rnnt_loss_test_py
98/111 Test #98: rnnt_loss_test_py ............................ Passed 2.77 sec
Start 99: shortest_path_test_py
99/111 Test #99: shortest_path_test_py ........................ Passed 5.00 sec
Start 100: sparse_abs_test_py
100/111 Test #100: sparse_abs_test_py ........................... Passed 1.96 sec
Start 101: symbol_table_test_py
101/111 Test #101: symbol_table_test_py ......................... Passed 1.18 sec
Start 102: top_sort_test_py
102/111 Test #102: top_sort_test_py ............................. Passed 1.88 sec
Start 103: union_test_py
103/111 Test #103: union_test_py ................................ Passed 1.93 sec
Start 104: replace_fsa_test_py
104/111 Test #104: replace_fsa_test_py .......................... Passed 2.19 sec
Start 105: Test.Cuda.cu_k2_torch_dense_fsa_vec_test
105/111 Test #105: Test.Cuda.cu_k2_torch_dense_fsa_vec_test ..... Passed 1.07 sec
Start 106: Test.Cuda.cu_k2_torch_deserialization_test
106/111 Test #106: Test.Cuda.cu_k2_torch_deserialization_test ... Passed 1.06 sec
Start 107: Test.Cuda.cu_k2_torch_fsa_class_test
107/111 Test #107: Test.Cuda.cu_k2_torch_fsa_class_test ......... Passed 1.05 sec
Start 108: Test.Cuda.cu_k2_torch_hypothesis_test
108/111 Test #108: Test.Cuda.cu_k2_torch_hypothesis_test ........ Passed 0.39 sec
Start 109: Test.Cuda.cu_k2_torch_parse_options_test
109/111 Test #109: Test.Cuda.cu_k2_torch_parse_options_test ..... Passed 0.52 sec
Start 110: Test.Cuda.cu_k2_torch_wave_reader_test
110/111 Test #110: Test.Cuda.cu_k2_torch_wave_reader_test ....... Passed 0.40 sec
Start 111: Test.torch_api_test
111/111 Test #111: Test.torch_api_test .......................... Passed 0.42 sec
99% tests passed, 1 tests failed out of 111
Total Test time (real) = 197.59 sec
The following tests FAILED:
10 - Test.Cuda.cu_hash_test (Failed)
Errors while running CTest
Output from these tests are in: /home/v-yifyang/k2/build_release/Testing/Temporary/LastTest.log
Use "--rerun-failed --output-on-failure" to re-run the failed cases verbosely.
make: *** [Makefile:71: test] Error 8 |
fix for flake8 fix for style check
Ready for merge. Could you help to check it? @csukuangfj |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a pad_left
boolean flag (default: false
) to the PadRagged
operation so users can choose left-side padding instead of the default right-side padding.
- Extend the C++
PadRagged
API and its Python bindings (pad
method andpad_ragged
function) with thepad_left
parameter. - Implement left-side padding logic in the
ragged_ops_inl.h
template for both constant and replicate modes. - Update tests to cover
pad_left=True
for integer and floating‐point ragged tensors in constant and replicate modes.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
k2/python/tests/ragged_ops_test.py | Added pad_left=True test cases for constant and replicate padding. |
k2/python/csrc/torch/v2/ragged_any.h | Updated RaggedAny::Pad signature to include pad_left parameter. |
k2/python/csrc/torch/v2/ragged_any.cu | Passed pad_left through to PadRagged and updated the binding. |
k2/python/csrc/torch/v2/any.cu | Added pad_left argument to the Pybind11 .def("pad", ...) call. |
k2/python/csrc/torch/ragged_ops.cu | Extended the pad_ragged free function binding with pad_left . |
k2/csrc/ragged_ops_inl.h | Implemented left‐padding code paths in PadRagged template. |
k2/csrc/ragged_ops.h | Updated PadRagged declaration to have a default pad_left=false . |
Comments suppressed due to low confidence (3)
k2/python/csrc/torch/v2/ragged_any.h:272
- Update the documentation comment for
Pad
to include the newpad_left
parameter, explaining its behavior and default value.
/// Wrapper for k2::PadRagged
k2/python/csrc/torch/v2/any.cu:363
- Ensure that the
kRaggedAnyPadDoc
docstring forpad
is updated to describe the newpad_left
argument and its default behavior.
py::arg("pad_left") = false, kRaggedAnyPadDoc);
k2/python/tests/ragged_ops_test.py:183
- Consider adding a test for the free function
pad_ragged(src, mode, padding_value, pad_left=True)
to verify thepad_left
parameter in the standalone API binding.
def test_remove_values_leq(self):
len = idx0x_next - idx0x, | ||
extra = col_num - len; | ||
if (pad_left) { | ||
if (j < extra) | ||
res_acc(i, j) = padding_value; | ||
else | ||
res_acc(i, j) = src_values_data[idx0x + j - extra]; | ||
} else { | ||
if (j >= len) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider renaming the variable len
to row_len
(and extra
to something like pad_len
) to improve readability and avoid shadowing standard functions.
len = idx0x_next - idx0x, | |
extra = col_num - len; | |
if (pad_left) { | |
if (j < extra) | |
res_acc(i, j) = padding_value; | |
else | |
res_acc(i, j) = src_values_data[idx0x + j - extra]; | |
} else { | |
if (j >= len) | |
row_len = idx0x_next - idx0x, | |
pad_len = col_num - row_len; | |
if (pad_left) { | |
if (j < pad_len) | |
res_acc(i, j) = padding_value; | |
else | |
res_acc(i, j) = src_values_data[idx0x + j - pad_len]; | |
} else { | |
if (j >= row_len) |
Copilot uses AI. Check for mistakes.
Can you add a test for an empty sublit list in the ragged tensor? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Left some minor comments. Otherwise, looks good to me.
k2/csrc/ragged_ops_inl.h
Outdated
@@ -794,7 +794,8 @@ Ragged<T> CreateRagged2(const std::vector<std::vector<T>> &vecs) { | |||
} | |||
|
|||
template <typename T> | |||
Array2<T> PadRagged(Ragged<T> &src, const std::string &mode, T padding_value) { | |||
Array2<T> PadRagged(Ragged<T> &src, const std::string &mode, T padding_value, | |||
bool pad_left) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bool pad_left) { | |
bool pad_left /* = false */) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
Yes, do these two cover the case? k2/k2/python/tests/ragged_ops_test.py Lines 107 to 114 in 70090e0
k2/k2/python/tests/ragged_ops_test.py Lines 128 to 130 in 70090e0
|
Sorry, you are right. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution!
This PR adds a new boolean parameter
pad_left
(default: false) to thePadRagged
function. When set to true, padding is applied to the left side of each sublist instead of the default right side.Test