Subdomain support for Interfaces + AllocCheck tests for iterators + Sparsitypattern bugfix #1108

AbdAlazezAhmed · 2024-11-07T21:56:42Z

fixes #1102, #1103, #1110, and maybe #850
Also a bit better performance

some random benchmark:

julia> function foo(dh, top)
           for i in InterfaceIterator(dh, top)
           end
           end
foo (generic function with 1 method)

julia> using BenchmarkTools

julia> grid = generate_grid(Hexahedron, (20,20,20))
Grid{3, Hexahedron, Float64} with 8000 Hexahedron cells and 9261 nodes

julia> topology = ExclusiveTopology(grid);

julia> ip = DiscontinuousLagrange{RefHexahedron, 1}();

julia> dh = DofHandler(grid);

julia> add!(dh, :u, ip);

julia> close!(dh);

julia<050f226>> @benchmark foo($dh, $topology)
BenchmarkTools.Trial: 2131 samples with 1 evaluation.
 Range (min … max):  1.960 ms …  28.916 ms  ┊ GC (min … max): 0.00% … 89.13%
 Time  (median):     2.151 ms               ┊ GC (median):    0.00%
 Time  (mean ± σ):   2.341 ms ± 739.360 μs  ┊ GC (mean ± σ):  1.66% ±  4.48%

   ▅▆▆█▅▄▃▁▁ ▂▃▄▃▅▅▄▃▃▃▂▂▁▁▁▁                                 ▁
  ▇█████████▇█████████████████▆▇▅▇▅▆▅▅▆▆▇▇█▆██▇▅▅▅▅▁▁▁▄▄▄▆▆▄▅ █
  1.96 ms      Histogram: log(frequency) by time      3.74 ms <

 Memory estimate: 1.86 MiB, allocs estimate: 42.

julia<master>> @benchmark foo($dh, $topology)
BenchmarkTools.Trial: 1605 samples with 1 evaluation.
 Range (min … max):  2.567 ms … 66.080 ms  ┊ GC (min … max): 0.00% … 93.99%
 Time  (median):     2.784 ms              ┊ GC (median):    0.00%
 Time  (mean ± σ):   3.109 ms ±  1.686 ms  ┊ GC (mean ± σ):  4.78% ±  9.39%

  ▇█▇▆▆▄▁   ▃▅▅▅▄▄▃▃▁ ▁                  ▁▂▁                 ▁
  █████████▇█████████▇█▇▄▆▄▆▆▄▆▄▅▁▅▇▆█▇███████▇▆▅▄▄▄▆▆▆▇▆▆▆▅ █
  2.57 ms      Histogram: log(frequency) by time     5.18 ms <

 Memory estimate: 2.99 MiB, allocs estimate: 124993.

codecov · 2024-11-08T16:53:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.69%. Comparing base (3b081be) to head (a38ed02).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1108      +/-   ##
==========================================
- Coverage   93.57%   92.69%   -0.89%     
==========================================
  Files          39       39              
  Lines        6071     6143      +72     
==========================================
+ Hits         5681     5694      +13     
- Misses        390      449      +59

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

Project.toml

AbdAlazezAhmed

Some points for feedback

AbdAlazezAhmed · 2024-11-13T19:27:23Z

src/Dofs/DofHandler.jl

@@ -12,7 +12,7 @@ Access some grid representation for the dof handler.
 """
 get_grid(dh::AbstractDofHandler)

-mutable struct SubDofHandler{DH} <: AbstractDofHandler
+mutable struct SubDofHandler{DH, CT} <: AbstractDofHandler


This makes us avoid dynamic dispatch for getting nodes/coords

AbdAlazezAhmed · 2024-11-13T19:28:22Z

src/Ferrite.jl

+"""
+An `InterfaceIndex` wraps an (Int, Int, Int, Int) and defines an interface by pointing to a (cell_here, facet_here, cell_there, facet_there).
+"""
+struct InterfaceIndex <: BoundaryIndex


This might be unnecessary but I think it's convenient to have it

AbdAlazezAhmed · 2024-11-13T19:28:44Z

src/Grid/utils.jl

@@ -165,6 +165,7 @@ function _create_boundaryset(f::Function, grid::AbstractGrid, top::ExclusiveTopo
            cell_idx = ff_nh_idx[1]
            facet_nr = ff_nh_idx[2]
            cell = getcells(grid, cell_idx)
+            length(facets(cell)) < facet_nr && continue


Fix for #1110

AbdAlazezAhmed · 2024-11-13T19:31:49Z

src/iterators.jl

+        flags::UpdateFlags = UpdateFlags()
+    )
+    grid = gridordh isa DofHandler ? get_grid(gridordh) : gridordh
+    set = Set(create_boundaryfacetset(grid, topology, _ -> true))


I'm using sets because ordered sets can allocate while iterating for some reason
And they do something a bit strange I'll have to check it again to make sure it's actually happening not just me hallucinating
They allocate on x86 but not on Arm :D

cc @fredrikekre any idea what is going on here?

Sounds very strange.. Needs some kind of repro to understand.

I believe they rehash (if necessary) before iteration.

So It turns out the alloc deference between arm and x86 was because I was testing the constructor too.
There's quite a big difference in nallocs for some reason with OrderedSet

a = Set(rand(Int, 1000)); @benchmark OrderedSet($a)

Arm (Snapdragon X Elite - X1E80100) WSL2 with the old kernel - generic arm cpu build:

BenchmarkTools.Trial: 10000 samples with 1 evaluation. Range (min … max): 19.900 μs … 115.683 ms ┊ GC (min … max): 0.00% … 99.66% Time (median): 29.100 μs ┊ GC (median): 0.00% Time (mean ± σ): 58.872 μs ± 1.163 ms ┊ GC (mean ± σ): 21.40% ± 1.69% █▇▆▂▁ ▅▆▆▆▃▂ ▂ █████████████▇▇▇▅▅▆▃▄▄▄▃▄▄▃▅▁▃▄▁▁▁▁▃▃▁▃▄▄▃▁▄▃▄▄▅▇▆▆▆▆▅▆▇▇▇▆▆ █ 19.9 μs Histogram: log(frequency) by time 298 μs < Memory estimate: 51.96 KiB, allocs estimate: 20.

x86 (i5-13500):

BenchmarkTools.Trial: 10000 samples with 1 evaluation. Range (min … max): 24.046 μs … 14.743 ms ┊ GC (min … max): 0.00% … 95.28% Time (median): 26.610 μs ┊ GC (median): 0.00% Time (mean ± σ): 34.303 μs ± 229.372 μs ┊ GC (mean ± σ): 11.95% ± 1.82% ▄▆██▇▅▄▄▄▄▄▃▂▂▁▂▂▁▁▁▁ ▁▁▁▁▁ ▁▁▁▁▁ ▂ ███████████████████████▇▇█▇▇▇█▆▆▆▅▅▅▅▅▆▇█████████████████▇▅▆ █ 24 μs Histogram: log(frequency) by time 55.5 μs < Memory estimate: 113.98 KiB, allocs estimate: 1052.

and it scales with the number of elements (for 10k elements -> arm:28 , x86: 10079)
And yes, rehashing seems to be the issue for static analysis :'D

AbdAlazezAhmed · 2024-11-13T19:32:32Z

test/test_grid_addboundaryset.jl

@@ -200,4 +200,21 @@
        addboundaryfacetset!(grid, topology, "test_boundary_facetset", filter_function)
        @test getfacetset(grid, "test_boundary_facetset") == Ferrite.create_boundaryfacetset(grid, topology, filter_function)
    end
+
+    @testset "addboundaryset Mixed grid" begin


test for #1110

AbdAlazezAhmed · 2024-11-13T19:34:50Z

test/test_interfacevalues.jl

+        for (sdh_here_idx, sdh_here) in enumerate(dh.subdofhandlers)
+            for (sdh_there_idx, sdh_there) in enumerate(dh.subdofhandlers)
+                sdh_there_idx < sdh_here_idx && continue
+                iv = InterfaceValues(
+                    qr_collection[sdh_here_idx], ip_collection[sdh_here_idx],
+                    qr_collection[sdh_there_idx], ip_collection[sdh_there_idx]
+                )
+                test_interfacevalues(
+                    iv, sdh_here, sdh_there
+                )
+            end
+        end


I'm thinking this might be the way to go for subdomains + interfaces? -until we get multicell/facet/interfacevalues-

AbdAlazezAhmed · 2024-11-25T19:03:33Z

#1100 should be merged first iirc. This one needs some feedback first esp with using Set instead of OrderedSet for performance

AbdAlazezAhmed added 4 commits November 7, 2024 21:04

homeoffice push

64eab2d

oopsie

050f226

use Set

ec96036

now tests shall pass

6f4f040

AbdAlazezAhmed added 11 commits November 8, 2024 18:51

runic

932b366

Init iterator with subdofhandlers

b14251e

oopsie

4659dba

now sets are moved from there to here

6044216

some tests

d0fc3b5

more tests

79d55f4

error paths + trying to test for allocs

9b1cbdd

Make Runic happy

be27c62

make alloccheck happy

cf7c08e

revert oopsie

0b68251

next: get rid of dynamic dispatches

19f11ea

KristofferC reviewed Nov 12, 2024

View reviewed changes

Project.toml Outdated Show resolved Hide resolved

AbdAlazezAhmed added 9 commits November 12, 2024 22:44

home office push

84a1a14

Bad workarounds, think about them tomorrow :P

261c50e

add cell type to SubDofHandler as typeparameter

4a22dea

don't enoforce no-allocs for iterators using DofHandler

cba4e1d

undo constrainthandler voodoo

5c6495e

tests for CellIterator

8f281d7

Construct FacetIterator without a set

7b97fed

Tests for FacetIterator

dcab007

Fix bug where boundary sets are broken for mixed grids

a38ed02

AbdAlazezAhmed mentioned this pull request Nov 13, 2024

create_boundary....set is broken for mixed grids #1110

Open

AbdAlazezAhmed added 3 commits November 13, 2024 19:12

move AllocCheck to test deps

94948c2

please

be9009a

enable interfacevalues test with mixed grid

0143341

AbdAlazezAhmed commented Nov 13, 2024

View reviewed changes

AbdAlazezAhmed changed the title ~~[WIP] Subdomain support for InterfaceIterator~~ Subdomain support for Interfaces + AllocCheck tests for iterators + Sparsitypattern bugfix Nov 13, 2024

avoid looping over interfaces twice for subdomains in sparsitypatterns

720d373

AbdAlazezAhmed marked this pull request as ready for review November 13, 2024 19:51

termi-official added the awaiting review PR is finished from the authors POV, waiting for feedback label Nov 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Subdomain support for Interfaces + AllocCheck tests for iterators + Sparsitypattern bugfix #1108

Subdomain support for Interfaces + AllocCheck tests for iterators + Sparsitypattern bugfix #1108

AbdAlazezAhmed commented Nov 7, 2024 •

edited

Loading

codecov bot commented Nov 8, 2024 •

edited

Loading

AbdAlazezAhmed left a comment

AbdAlazezAhmed Nov 13, 2024

AbdAlazezAhmed Nov 13, 2024

AbdAlazezAhmed Nov 13, 2024

AbdAlazezAhmed Nov 13, 2024 •

edited

Loading

termi-official Nov 13, 2024

KristofferC Nov 13, 2024

fredrikekre Nov 13, 2024

AbdAlazezAhmed Nov 14, 2024

AbdAlazezAhmed Nov 13, 2024

AbdAlazezAhmed Nov 13, 2024

AbdAlazezAhmed commented Nov 25, 2024

Subdomain support for Interfaces + AllocCheck tests for iterators + Sparsitypattern bugfix #1108

Are you sure you want to change the base?

Subdomain support for Interfaces + AllocCheck tests for iterators + Sparsitypattern bugfix #1108

Conversation

AbdAlazezAhmed commented Nov 7, 2024 • edited Loading

codecov bot commented Nov 8, 2024 • edited Loading

Codecov Report

AbdAlazezAhmed 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

AbdAlazezAhmed Nov 13, 2024 • edited Loading

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

AbdAlazezAhmed commented Nov 25, 2024

AbdAlazezAhmed commented Nov 7, 2024 •

edited

Loading

codecov bot commented Nov 8, 2024 •

edited

Loading

AbdAlazezAhmed Nov 13, 2024 •

edited

Loading