Fix 2D/3D L1 error quadrature sampling only the diagonal#14
Merged
Conversation
simple_quad passes the whole array of quadrature points to f at once. In the nested 2D/3D calls the outer lambda therefore received an array of y points, and analytical([x, y]) paired x_i with y_i elementwise, so the cell integral was evaluated only along the cell diagonal and then rescaled by the weight sum. For f(x,y) = x*y over the unit square this gives 1/3 instead of 1/4. Replace the nested calls with tensor-product quadrature helpers. The analytical callable still receives equal length coordinate arrays.
Contributor
Author
|
Added a small standalone regression test in |
The existing checks use square cells and integrands that are symmetric in x and y, so a swapped argument pair or a mixed up axis would still pass. Add direct checks of simple_quad_2d and simple_quad_3d on asymmetric boxes against closed form integrals.
Contributor
|
This is a very useful patch. Thank you for spotting the error, doing the verification work and adding a test. |
Contributor
Author
Thankyou for your kind response. I appreciate it and I hope to learn and contribute more . Honestly was good fun this one. |
JulienLoiseau
approved these changes
Jul 13, 2026
JulienLoiseau
approved these changes
Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This fixes the 2D and 3D L1 error calculation in
compute_l1_error_fvm.The old nested
simple_quadcalls passed whole quadrature-point arrays into the analytical function. In 2D and 3D that pairedx_iwithy_iandz_i, so the integral was sampled along the cell diagonal and then scaled by the weight sum.For example, using the old path:
integral x*y dx dyover[0, 1]^2gives1/3, but the correct value is1/4.integral x*y*z dx dy dzover[0, 1]^3gives1/4, but the correct value is1/8.The patch adds explicit tensor-product Gauss-Legendre helpers for 2D and 3D and keeps the analytical callable contract the same: it still receives equal-length coordinate arrays.
Checks
tools/test_verify_lib.pywith 2D and 3D exact-cell-average regression tests.python tools/test_verify_lib.pysin(x + y)against its exact 2D integral.0.git diff --checkruff check tools/verify_lib.py tools/test_verify_lib.py