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.

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

Numpy fill accepts only numbers #1389

Closed
TizianoDeMatteis opened this issue Oct 12, 2023 · 2 comments · Fixed by #1420
Closed

Numpy fill accepts only numbers #1389

TizianoDeMatteis opened this issue Oct 12, 2023 · 2 comments · Fixed by #1420
Assignees

Comments

@TizianoDeMatteis
Copy link
Contributor

Describe the bug

Current replacement for numpy.fill checks that the value is a Number (or boolean). This prevents using a variable.

To Reproduce

import dace
import numpy as np

N = dace.symbol("N", dace.int32)


@dace.program
def fill_dace(A: dace.float64[N, N], value: dace.float64):
    A.fill(value)


N = 8
value = 0.3
input = np.ndarray((N, N)).astype(np.float64)
input_ref = np.copy(input)

fill_dace(input, value)
input_ref.fill(value)
assert np.allclose(input, input_ref)

Steps to reproduce the behavior:

  1. Run the code above
  2. It will return a DaceSyntaxError

Expected behavior
Allow also variables (or parameters) to be passed to fill

@philip-paul-mueller philip-paul-mueller self-assigned this Oct 31, 2023
@philip-paul-mueller
Copy link
Collaborator

I will take a look at it.

@philip-paul-mueller
Copy link
Collaborator

philip-paul-mueller commented Nov 1, 2023

The reason for this behaviour is located in _elementwise() (to which fill() is forwarded).
It directly creates a very simple map with a single tasklet.
I think that this rerstriction is because it could lead to arbitrary complex map internals otherwise.

I think that a "simple" maybe not good way would be to rewrite it to an assignement.

philip-paul-mueller added a commit to philip-paul-mueller/dace that referenced this issue Nov 1, 2023
This changes makes it possible that the `fill()` function accepts variables.
However, this particular commit only allows variables that where passed as arguments, at least in some sense, see the discussion of the issue.

This is a first step to address issue [spcl#1389](spcl#1389).
@BenWeber42 BenWeber42 linked a pull request Nov 15, 2023 that will close this issue
BenWeber42 added a commit that referenced this issue Nov 17, 2023
This PR is for addressing issue
[#1389](#1389).

---------

Co-authored-by: acalotoiu <61420859+acalotoiu@users.noreply.github.com>
Co-authored-by: BenWeber42 <dev.ben.weber@gmail.com>
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 a pull request may close this issue.

2 participants