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

Upgrade PartitionedArrays from 0.3.x to 0.5.x #18

Merged
merged 1 commit into from
Sep 28, 2024
Merged

Conversation

fredrikekre
Copy link
Owner

No description provided.

Copy link
Owner Author

@fredrikekre fredrikekre left a comment

Choose a reason for hiding this comment

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

Also wanted to ask if using .row_partition and .col_partition like in

map(local_values(B), B.row_partition, B.col_partition) do Bv, Br, Bc
is the correct way?


# Extract sparse matrices from the SplitMatrix. We are only interested in the owned
# rows, so only consider own-own and own-ghost blocks.
Aoo = A.blocks.own_own::SparseMatrixCSC
Copy link
Owner Author

Choose a reason for hiding this comment

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

@fverdugo is using A.block.own_own the correct way or is there some function to extract this part? I saw https://github.com/fverdugo/PartitionedArrays.jl/blob/b211949cc5842f3d6d99f8452b9f0233aef963a5/src/p_sparse_matrix.jl#L670-L672 but not sure what to pass as the second and third argument (although they are unused so doesn't matter...)

@fredrikekre
Copy link
Owner Author

Also wanted to ask if using .row_partition and .col_partition like in

https://github.com/fverdugo/PartitionedArrays.jl/blob/b211949cc5842f3d6d99f8452b9f0233aef963a5/src/p_sparse_matrix.jl#L993 looks close. partition(axes(A, 1)) perhaps? Would it make sense to have something like local_axes(A)?

Copy link

codecov bot commented Sep 28, 2024

Codecov Report

Attention: Patch coverage is 82.19178% with 13 lines in your changes missing coverage. Please review.

Project coverage is 76.78%. Comparing base (9d35b75) to head (01e91b1).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/HYPRE.jl 82.19% 13 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #18      +/-   ##
==========================================
- Coverage   77.69%   76.78%   -0.92%     
==========================================
  Files           4        4              
  Lines        1139     1163      +24     
==========================================
+ Hits          885      893       +8     
- Misses        254      270      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This patch upgrades the PartitionedArrays dependency from the 0.3.x
release series to 0.5.x (closes #17). Also updates documentation build
dependencies.
@fredrikekre fredrikekre merged commit 90a92d7 into master Sep 28, 2024
9 checks passed
@fredrikekre fredrikekre deleted the fe/parrays-0.4 branch September 28, 2024 22:12
@fverdugo
Copy link

Nice to see this merged so quickly!

Also wanted to ask if using .row_partition and .col_partition like in

These fields are documented (thus public), but I always use partition(axes(A,x)) outside PAs.

NB. AFAIK, Petsc (and provably Hypre) assumes that the global row ids are partitioned using a "block" partition (consecutive rows go to the same rank) and blocks are mapped to ranks in ascending rank id. Moreover ghost columns are sorted according to global id. None of these restrictions apply to PAs, which makes challenging to convert a PSparseMatrix to a Petsc matrix since you need to reformat data to match Petsc assumptions.

Take a look to this function to check how I am building a Petsc matrix from a PSparseMatrix:

https://github.com/fverdugo/PetscCall.jl/blob/3602086520882797b0210df22fabdb5af9a46a5a/src/ksp.jl#L246

You provably need to do something similar (unless you are already doing it) so that HYPRE.jl is able to work with any PSparseMatrix.

@fredrikekre
Copy link
Owner Author

Yea, right now I am just throwing an error instead (

@assert iupper - ilower + 1 == length(own_to_global_row)
).

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.

2 participants