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

calc_boundary_flux! is allocating for HOHQMesh and DGMulti #1678

Closed
DanielDoehring opened this issue Oct 20, 2023 · 1 comment · Fixed by #1695
Closed

calc_boundary_flux! is allocating for HOHQMesh and DGMulti #1678

DanielDoehring opened this issue Oct 20, 2023 · 1 comment · Fixed by #1695
Assignees
Labels
performance We are greedy

Comments

@DanielDoehring
Copy link
Contributor

DanielDoehring commented Oct 20, 2023

While tackling #1664 I observed that

# "lispy tuple programming" instead of for loop for type stability
function calc_boundary_flux!(cache, t, boundary_conditions, mesh,
have_nonconservative_terms, equations, dg::DGMulti)
# peel off first boundary condition
calc_single_boundary_flux!(cache, t, first(boundary_conditions),
first(keys(boundary_conditions)),
mesh, have_nonconservative_terms, equations, dg)
# recurse on the remainder of the boundary conditions
calc_boundary_flux!(cache, t, Base.tail(boundary_conditions),
mesh, have_nonconservative_terms, equations, dg)
end

is allocating for the dgmulti_2d/elixir_euler_hohqmesh.jl example, see e.g. https://github.com/trixi-framework/Trixi.jl/actions/runs/6574426931/job/17859451194#step:7:8803

I could not figure out why that is, also the specialization

# "lispy tuple programming" instead of for loop for type stability
function calc_boundary_flux!(cache, t, boundary_conditions::BC, mesh,
                             have_nonconservative_terms, equations, dg::DGMulti) where {BC}

    # peel off first boundary condition
    calc_single_boundary_flux!(cache, t, first(boundary_conditions),
                               first(keys(boundary_conditions)),
                               mesh, have_nonconservative_terms, equations, dg)

    # recurse on the remainder of the boundary conditions
    calc_boundary_flux!(cache, t, Base.tail(boundary_conditions),
                        mesh, have_nonconservative_terms, equations, dg)
end

does not solve the issue.

@DanielDoehring DanielDoehring added the performance We are greedy label Oct 20, 2023
@jlchan jlchan self-assigned this Oct 20, 2023
@jlchan
Copy link
Contributor

jlchan commented Oct 31, 2023

Awesome! Glad #1695 fixes it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance We are greedy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants