Skip to content

Add fill() generator that yields a user-provided value at all indices#1162

Open
tbensonatl wants to merge 4 commits intomainfrom
feature/add-fill-generator
Open

Add fill() generator that yields a user-provided value at all indices#1162
tbensonatl wants to merge 4 commits intomainfrom
feature/add-fill-generator

Conversation

@tbensonatl
Copy link
Copy Markdown
Collaborator

New matx::fill(shape, value) generator that returns an operator that yields the same value at every index. Modeled on ones/zeros, but with a caller-supplied value and no default for T — the type is deduced from value or specified explicitly. This avoids the case of a default type (e.g., int) causing unwanted conversions or truncations of the user-provided value.

New matx::fill(shape, value) generator that returns an operator that yields
the same value at every index. Modeled on ones/zeros, but with a caller-supplied
value and no default for T — the type is deduced from value or specified
explicitly. This avoids the case of a default type (e.g., int) causing
unwanted conversions or truncations of the user-provided value.

Signed-off-by: Thomas Benson <tbenson@nvidia.com>
@tbensonatl tbensonatl self-assigned this Apr 25, 2026
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Apr 25, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@cliffburdick
Copy link
Copy Markdown
Collaborator

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 25, 2026

Greptile Summary

This PR adds matx::fill(shape, value), a new generator that wraps detail::ConstVal to return a user-supplied constant at every index. It is cleanly modeled on ones/zeros with four overloads (shape-taking, C-array, rank-0 empty-brace, and shapeless), intentionally omitting a default for T to prevent silent narrowing conversions.

Confidence Score: 5/5

Safe to merge; the single finding is a P2 documentation label mismatch that does not affect runtime behavior.

All logic is correct and consistent with existing ones/zeros patterns. The only issue is a misleading section heading in the .rst file — the rendered example for 'Shapeless form' actually shows the ranked form.

docs_input/api/creation/operators/fill.rst — section label for fill-gen-test-2 needs correction.

Important Files Changed

Filename Overview
include/matx/generators/fill.h Adds four fill() overloads (shape-taking, array-shape, rank-0 empty-brace, shapeless) wrapping detail::ConstVal; correctly modeled on ones/zeros with no default for T; implementation is clean and correct.
docs_input/api/creation/operators/fill.rst Documents all four overloads with doxygenfunction directives; the fill-gen-test-2 example section label ("Shapeless form…") incorrectly describes the shape-taking ranked form shown in the code extract.
include/matx/generators/generators.h Single include added in alphabetical order; no issues.
test/00_operators/GeneratorTests.cu Adds five typed/non-typed tests covering rank-1, shapeless, rank-0 (array and empty-brace), and zipvec usage; coverage is thorough and test logic is correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["fill(shape, value) call site"] --> B{Overload resolution}
    B -->|"ShapeType&& (non-array)"| C["fill(ShapeType&&, T value)"]
    B -->|"C-style array index_t[RANK]"| D["fill(const index_t(&)[RANK], T value)"]
    B -->|"Empty brace {}"| E["fill(initializer_list<no_size_t>, T value)"]
    B -->|"No shape argument"| F["fill(T value)"]
    C --> G["ConstVal<T, ShapeType>(s, value)"]
    D -->|"to_array(s)"| C
    E -->|"array<index_t,0>{}"| C
    F -->|"NoShape{}"| C
    G --> H["Rank() = tuple_size<ShapeType> OR matxNoRank"]
Loading

Reviews (4): Last reviewed commit: "Add better-motivated examples for the fi..." | Re-trigger Greptile

Comment on lines +20 to +22
.. doxygenfunction:: matx::fill(ShapeType &&s, T value)
.. doxygenfunction:: matx::fill(const index_t (&s)[RANK], T value)
.. doxygenfunction:: matx::fill(T value)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Missing doxygen directive for rank-0 empty-brace overload

The .rst file documents three of the four public overloads, but the rank-0 empty-brace form fill(const std::initializer_list<detail::no_size_t>, T value) has no doxygenfunction directive. The prose and fill-gen-test-3 example cover the usage, but the auto-generated API reference table will omit that overload signature. Consider adding:

.. doxygenfunction:: matx::fill(const std::initializer_list<detail::no_size_t>, T value)

@tbensonatl
Copy link
Copy Markdown
Collaborator Author

tbensonatl commented Apr 25, 2026

Isn't this the same as https://github.com/NVIDIA/MatX/blob/main/include/matx/operators/constval.h?

Yes. This is just syntactic sugar over ConstVal -- it returns a ConstVal. Unless I missed the operator/generator wrapper, users would need to do something like:

matx::detail::ConstVal<float, cuda::std::array<index_t, 1>> cv( cuda::std::array<index_t, 1>{100}, 3.14f);

to make their own ConstVal. We could call this matx::constval. I just used fill to align with numpy naming conventions.

Signed-off-by: Thomas Benson <tbenson@nvidia.com>
@tbensonatl
Copy link
Copy Markdown
Collaborator Author

/build

Copy link
Copy Markdown
Collaborator

@cliffburdick cliffburdick left a comment

Choose a reason for hiding this comment

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

@tbensonatl is this effectively the same as just using a constant, but in situations where you may want the shape to be different? I'm looking at the tests and some of them can just be:

(A = 3.f).run();

The constant should adapt it's shape to anything on the lhs or rhs.

@tbensonatl
Copy link
Copy Markdown
Collaborator Author

@tbensonatl is this effectively the same as just using a constant, but in situations where you may want the shape to be different? I'm looking at the tests and some of them can just be:

(A = 3.f).run();

The constant should adapt it's shape to anything on the lhs or rhs.

Yes, in that case it could just be a literal. The motivating case is that greptile is unhappy that I dropped support for a scalar argument input for the sar_bp operator so that it now requires either a rank-0 or rank-1 tensor. I mainly did that to clean up the sar_bp kernel code. This would basically be a drop-in replacement where users can now pass ..., fill({}, range_to_mcp), ... (range_to_mcp is just a run-time scalar value). Previously, the operator supported scalars, 0D tensors, and 1D tensors, but fill can play the role of a 0D tensor, and we can drop the scalar-input path.

Signed-off-by: Thomas Benson <tbenson@nvidia.com>
@tbensonatl
Copy link
Copy Markdown
Collaborator Author

/build

@tbensonatl
Copy link
Copy Markdown
Collaborator Author

The constant should adapt it's shape to anything on the lhs or rhs.

Alternatively, if we only want to cover the scalar-to-rank-0 operator case, then we could just add something like scalar_as_op() for just that case. The other cases that I can think of where fill() may be useful would be if you want a shape-carrying scalar versus just using a scalar that will broadcast to any size. fill() also works in other contexts where an operator is expected and regular scalars are not supported (e.g., as an input to zipvec).

Signed-off-by: Thomas Benson <tbenson@nvidia.com>
@tbensonatl
Copy link
Copy Markdown
Collaborator Author

/build

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage is 91.788%feature/add-fill-generator into main. No base build found for main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants