Allow uniform_partition to have arbitrarily many ghost layers #173

Joroks · 2024-09-22T20:43:29Z

I've modified uniform_partition such that it now works with arbitrarily thick ghost layers. This would be a first step towards #97.
On my machine, it passes all the tests.

fverdugo · 2024-09-23T14:52:23Z

@Joroks Thanks for this PR. I will need some time to review this. I will come back to you.

fverdugo

@Joroks Thanks for this PR! Please see my comments.

fverdugo · 2024-09-29T08:14:23Z

src/p_range.jl

@@ -552,7 +552,7 @@ interface.
 -  `periodic::NTuple{N}=ntuple(i->false,N)`: Use or not periodic boundaries per direction.

 For convenience, one can also provide scalar inputs instead tuples
-to create 1D block partitions. In this case, the argument `np` can be omitted
+to create 1D block partitions. In this case, the argument `np` is omitted


This looks like a breaking change. Is it really needed? Please keep the current option of not omitting np.

I had to remove this option because this would otherwise result i a duplicate function signature.

uniform_partition(rank,n::Integer,ghost::Integer,periodic::Bool=false)

would be the same as

uniform_partition(rank,np::Integer,n::Integer)

if periodic is not specified.

I'm open to other ideas on how to handle this problem

Hi @Joroks,

good point. I see the problem.

Maybe we can add a new method in which the optional arguments are key-word arguments:

function uniform_partition(ranks,np::Tuple,n::Tuple; ghost=map(i->false,np),periodic=map(i->false,np)) end

This one should not interfere with any of the existing methods, I would say.

And for the moment build the thick ghost layer only using always tuples, also in the 1d case.

In the next breaking release, we can polish all these methods.

The reason for this is that I don't want to introduce breaking changes at this moment.

Hi @fverdugo,

the original type signature doesn't restrict the ghost argument to be a tuple of booleans.
I don't think it's necessary to use the key-word arguments to avoid breaking changes.
I've updated to documentation and reverted the function signature to reflect that.

fverdugo · 2024-09-29T08:17:34Z

src/p_range.jl

    lis = CircularArray(LinearIndices(n))
    local_cis = CartesianIndices(local_ranges)
-    owner_lis = CircularArray(LinearIndices(np))


I think this was the only place where we used CircularArrays (please confirm). If so, can you remove it as a dependence?

There is still another place where CircularArrays are used (two lines above actually).

fverdugo · 2024-09-29T08:23:59Z

src/p_range.jl

@@ -809,39 +800,32 @@ function renumber_partition(partition_in;renumber_local_indices=true)
 end

 function local_range(p,np,n,ghost=false,periodic=false)


Is the new version of this function equivalent to the previous one? In the previous, we distribute the reminder by giving an extra item to the first parts (or the last parts, now not I don't remember). Can you please include (by replying this comment) the output of this new version for some illustrative input values?

The logic on how to distribute the remainder hasn't changed as offset is still calculated exactly the same way. I just simplified the function a bit and made it work with more than one layer of ghost atoms.

julia> [PartitionedArrays.local_range(i, 5, 17) for i in 1:5]' 1×5 adjoint(::Vector{UnitRange{Int64}}) with eltype LinearAlgebra.Adjoint{Int64, UnitRange{Int64}}: [1 2 3] [4 5 6] [7 8 9] [10 11 12 13] [14 15 16 17] julia> [PartitionedArrays.local_range(i, 5, 17, true) for i in 1:5]' 1×5 adjoint(::Vector{UnitRange{Int64}}) with eltype LinearAlgebra.Adjoint{Int64, UnitRange{Int64}}: [1 2 3 4] [3 4 … 6 7] [6 7 … 9 10] [9 10 … 13 14] [13 14 … 16 17] julia> [PartitionedArrays.local_range(i, 5, 17, true, true) for i in 1:5]' 1×5 adjoint(::Vector{UnitRange{Int64}}) with eltype LinearAlgebra.Adjoint{Int64, UnitRange{Int64}}: [0 1 … 3 4] [3 4 … 6 7] [6 7 … 9 10] [9 10 … 13 14] [13 14 … 17 18]

fverdugo · 2024-09-29T08:24:20Z

test/p_range_tests.jl

@@ -51,6 +51,22 @@ function p_range_tests(distribute)
   periodic = (true,true)
   uniform_partition(rank,np,n,ghost,periodic)

+   # uniform Cartesian partition with two layers of ghosts


Good to add tests. Thanks!

fverdugo · 2024-10-07T14:50:00Z

@Joroks thanks for the last changes. Let's wait for the checks to pass.

fverdugo · 2024-10-07T14:52:09Z

@Joroks out of curiosity: Is there a way I can find your personal webpage / google scholar?

Joroks · 2024-10-07T15:06:05Z

@fverdugo I unfortunately don't have any because im currently still only a bachelor student. I'm currently in the process of finishing my bachelors thesis which is related to running simulations on HPC clusters which has gotten me interested large scale parallel programms through the use of MPI and julia. I was the person suggesting to migrate the simulation code from matlab to julia, which is the topic of my thesis, alongside some mathematical improvements i discovered along the way.

I can however point you to the research group I'm writing my bachelors thesis for: https://www.capriccio.research.fau.eu/

fverdugo · 2024-10-07T15:17:01Z

@Joroks thanks for the link!

Joroks added 5 commits September 22, 2024 21:06

allow for arbitrary number of ghosts

d38bf59

fixup! function signature

c538e83

fixup! faster algorithm

8d72369

comment out unused and out of date code

6498a0d

add tests

7509578

Joroks changed the title ~~Nghosts~~ Allow uniform_partition to have arbitrarily many ghost layers Sep 22, 2024

fverdugo self-requested a review September 23, 2024 14:52

fverdugo requested changes Sep 29, 2024

View reviewed changes

Joroks added 2 commits October 7, 2024 15:11

revert function signature

b99e045

update documentation

d322a98

fverdugo approved these changes Oct 7, 2024

View reviewed changes

fverdugo merged commit a948ae4 into fverdugo:master Oct 7, 2024
4 checks passed

fverdugo mentioned this pull request Oct 7, 2024

Partitioned multidimensional arrays (generalize PVector into PArray) #97

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow uniform_partition to have arbitrarily many ghost layers #173

Allow uniform_partition to have arbitrarily many ghost layers #173

Joroks commented Sep 22, 2024 •

edited

Loading

fverdugo commented Sep 23, 2024

fverdugo left a comment

fverdugo Sep 29, 2024

Joroks Sep 29, 2024

fverdugo Sep 30, 2024

fverdugo Sep 30, 2024

Joroks Oct 7, 2024

fverdugo Sep 29, 2024

Joroks Sep 29, 2024

fverdugo Sep 29, 2024 •

edited

Loading

Joroks Sep 29, 2024

fverdugo Sep 29, 2024

fverdugo commented Oct 7, 2024

fverdugo commented Oct 7, 2024

Joroks commented Oct 7, 2024

fverdugo commented Oct 7, 2024

		@@ -809,39 +800,32 @@ function renumber_partition(partition_in;renumber_local_indices=true)
		end

		function local_range(p,np,n,ghost=false,periodic=false)

Allow uniform_partition to have arbitrarily many ghost layers #173

Allow uniform_partition to have arbitrarily many ghost layers #173

Conversation

Joroks commented Sep 22, 2024 • edited Loading

fverdugo commented Sep 23, 2024

fverdugo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fverdugo Sep 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fverdugo commented Oct 7, 2024

fverdugo commented Oct 7, 2024

Joroks commented Oct 7, 2024

fverdugo commented Oct 7, 2024

Joroks commented Sep 22, 2024 •

edited

Loading

fverdugo Sep 29, 2024 •

edited

Loading