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

Passing views for copy!() to/from HYPREVector? #34

Open
johnomotani opened this issue Feb 3, 2025 · 4 comments
Open

Passing views for copy!() to/from HYPREVector? #34

johnomotani opened this issue Feb 3, 2025 · 4 comments

Comments

@johnomotani
Copy link

I wanted to be able to pass a view to copy!(), but at the moment this is not possible, because the arguments require a Vector{HYPRE_Complex} here

HYPRE.jl/src/HYPRE.jl

Lines 221 to 248 in faa61cc

function Base.copy!(dst::Vector{HYPRE_Complex}, src::HYPREVector)
ilower, iupper = Internals.get_proc_rows(src)
nvalues = iupper - ilower + 1
if length(dst) != nvalues
throw(ArgumentError("length of dst and src does not match"))
end
indices = collect(HYPRE_BigInt, ilower:iupper)
@check HYPRE_IJVectorGetValues(src, nvalues, indices, dst)
return dst
end
function Base.copy!(dst::HYPREVector, src::Vector{HYPRE_Complex})
ilower, iupper = Internals.get_proc_rows(dst)
nvalues = iupper - ilower + 1
if length(src) != nvalues
throw(ArgumentError("length of dst and src does not match"))
end
# Re-initialize the vector
@check HYPRE_IJVectorInitialize(dst)
# Set all the values
indices = collect(HYPRE_BigInt, ilower:iupper)
@check HYPRE_IJVectorSetValues(dst, nvalues, indices, src)
# TODO: It shouldn't be necessary to assemble here since we only set owned rows (?)
# @check HYPRE_IJVectorAssemble(dst)
# TODO: Necessary to recreate the ParVector? Running some examples it seems like it is
# not needed.
return dst
end

Would it be possible to make this AbstractVector instead? If it's correct/legal, I can make a PR to make the change and add a test, but I don't know enough about ccall to know if this is sensible.

@johnomotani
Copy link
Author

I'd be interested in the case when the views are associated with a contiguous piece of memory, I guess it might be necessary to restrict to that in order to pass a pointer to C? Maybe there is a way to restrict the type signature of a SubArray to enforce this?

@fredrikekre
Copy link
Owner

I think StridedArray may be the closest, but that allows non-1 strides so not sure it would work. There was some discussion in #16 (comment) but I can't seem to trigger the failure discussed there with non-contiguous views.

@johnomotani
Copy link
Author

It looks like #16 was talking about PartitionedArrays specifically? I was trying to use the 'Direct assembly' method. Just trying to clarify because I don't know if it's (exactly?) the same problem.

@fredrikekre
Copy link
Owner

It was about passing views from PartitionedArrays. Anyway, I think you can relax the signature to StridedVector and then we add a runtime @assert stride(x, 1) == 1.

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

2 participants