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 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

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

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 7, 2024 •

edited

Loading

codecov bot commented Nov 8, 2024 •

edited

Loading

AbdAlazezAhmed Nov 13, 2024 •

edited

Loading