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

Use enabled_coefficients to restrict packing of coefficients #3260

Merged
merged 24 commits into from
Jun 9, 2024

Conversation

jorgensd
Copy link
Member

@jorgensd jorgensd commented Jun 7, 2024

Pack only entities used in integral type:
MWE:

from mpi4py import MPI
import dolfinx
import ufl

mesh = dolfinx.mesh.create_unit_cube(MPI.COMM_WORLD, 50, 50, 50)
V = dolfinx.fem.functionspace(mesh, ("Lagrange", 3))
N = 50
us = [dolfinx.fem.Function(V) for _ in range(N)]
for i,u in enumerate(us):
    u.x.array[:] = i + 1
    u.name=f"u_{i}"

volume_form = us[0] * ufl.dx
surface_form = sum(us[i] * ufl.ds for i in range(N))

combined_form = volume_form + surface_form
compiled_form = dolfinx.fem.form(combined_form)

import time
start = time.perf_counter()
coeffs = dolfinx.fem.assemble.pack_coefficients(compiled_form._cpp_object)
end =time.perf_counter()
print(f"{dolfinx.common.git_commit_hash=} {end-start=:.5e}")
print(coeffs[(dolfinx.fem.IntegralType.cell, -1)])

On the main branch, all coefficients are packed for the cell integral, even if only the first coefficient is used in that integral.
Runtime on this branch:

dolfinx.common.git_commit_hash='002e1fa9360995f9c71414a683e7ce618d6101e9' end-start=1.70250e+00

on main

dolfinx.common.git_commit_hash='64e35310085683f4f61804e70b8b58b9bb8653ae' end-start=2.89335e+00

It would also resolve #3256 and simplify some logic in: #3224

@jorgensd jorgensd changed the title Add enabled_coefficients to integral data to avoid packing too much Use enabled_coefficients to restrict packing of coefficients Jun 7, 2024
@jorgensd jorgensd added bug Something isn't working performance labels Jun 7, 2024
@jorgensd jorgensd marked this pull request as ready for review June 7, 2024 09:56
cpp/dolfinx/fem/Form.h Outdated Show resolved Hide resolved
cpp/dolfinx/fem/Form.h Outdated Show resolved Hide resolved
cpp/dolfinx/fem/Form.h Outdated Show resolved Hide resolved
cpp/dolfinx/fem/Form.h Outdated Show resolved Hide resolved
cpp/dolfinx/fem/Form.h Outdated Show resolved Hide resolved
cpp/dolfinx/fem/utils.h Outdated Show resolved Hide resolved
cpp/dolfinx/fem/utils.h Outdated Show resolved Hide resolved
@jorgensd jorgensd requested a review from garth-wells June 8, 2024 14:16
@jorgensd
Copy link
Member Author

jorgensd commented Jun 9, 2024

Running the test above in serial on my system:
Main:

dolfinx.common.git_commit_hash='90e4fee36c2f5d1254ceec4638fd4194cc8be7da' end-start=2.90846e+01

This PR

dolfinx.common.git_commit_hash='9f19213e8b4d6e586601dd8d1ce40b37da948aa1' end-start=6.47885e+00

@jorgensd jorgensd enabled auto-merge June 9, 2024 19:39
@jorgensd jorgensd added this pull request to the merge queue Jun 9, 2024
Merged via the queue into main with commit 46c7628 Jun 9, 2024
28 checks passed
@jorgensd jorgensd deleted the dokken/restrict_packing branch June 9, 2024 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high-priority performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]: Packing of codim-0 form not working for mixture of measures
2 participants