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

Bug in dense_dims for reshaped views #157

Open
ranocha opened this issue May 31, 2021 · 4 comments
Open

Bug in dense_dims for reshaped views #157

ranocha opened this issue May 31, 2021 · 4 comments

Comments

@ranocha
Copy link
Contributor

ranocha commented May 31, 2021

Reported in #156 (comment):

julia> using ArrayInterface

julia> u_base = randn(10, 10); u_view = view(u_base, 3, :); u_reshaped_view = reshape(u_view, 1, size(u_base, 2));

julia> ArrayInterface.dense_dims(u_reshaped_view)

As confirmed by @chriselrod in #156 (comment), this is a bug:

The correct answer for u_reshaped_view would be (False(),False()).

@Tokazama
Copy link
Member

resolved with #161

@ranocha
Copy link
Contributor Author

ranocha commented Jun 10, 2022

I fixed it only for the case of reshaped versions of abstract vectors. It still fails for reshaped views of higher-dimensional arrays, e.g.,

(@v1.7) pkg> activate --temp
  Activating new project at `/tmp/jl_4vH1Gz`

(jl_4vH1Gz) pkg> add ArrayInterface
    Updating registry at `~/.julia/registries/General.toml`
   Resolving package versions...
    Updating `/tmp/jl_4vH1Gz/Project.toml`
  [4fba245c] + ArrayInterface v6.0.14

julia> using ArrayInterface

julia> u_base = randn(10, 10, 10); u_view = view(u_base, 3, :, :); u_reshaped_view = reshape(u_view, 1, size(u_base, 2) * size(u_base, 3));

julia> ArrayInterface.dense_dims(u_reshaped_view)

julia> ArrayInterface.dense_dims(u_reshaped_view) |> isnothing
true

Thus, this issue is not resolved in general.

@Tokazama
Copy link
Member

I guess in that example we could know that no dimensions are dense because u_view doesn't have any dense dimensions, but I'm not sure we can do any better at identifying instances where they are dense.

@Tokazama
Copy link
Member

#307 tests all the examples in this issue. I'm assuming there are still times when we don't detect dense dimensions just because we can't know in some situations without knowing the size.

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

No branches or pull requests

3 participants