-
Notifications
You must be signed in to change notification settings - Fork 22
Comparison against the 'py-pde' package based on the example: Diffusion equation with spatial dependence #578
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
base: main
Are you sure you want to change the base?
Changes from all commits
3129b9c
9cc4c21
75e566b
f358075
3e694cd
155db14
32fc63e
7617ea2
764c24b
8601efe
89347a7
6bdf368
06af16d
5dab1eb
aceeb76
ca8516a
d840f92
9daeb12
373636f
3a1c9b1
1db01be
3153dc0
10d674f
196904d
ca6abe0
6321be4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handling of edge cases (such as what happens if the diffusivity profile contains zeros or negative values) isn’t explicitly addressed—consider adding input validation or at least a warning to ensure consistent behavior for all user inputs. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,7 +43,56 @@ def apply(traversals_data, advector, advectee): | |
null_vecfield_bc, | ||
*null_scalarfield, | ||
null_scalarfield_bc, | ||
traversals_data.buffer | ||
traversals_data.buffer, | ||
) | ||
|
||
return apply | ||
|
||
|
||
def make_heterogeneous_laplacian( | ||
non_unit_g_factor: bool, options: Options, traversals: Traversals | ||
): | ||
"""returns njit-ted function for heterogeneous diffusion with spatially varying diffusivity | ||
|
||
Note: heterogeneous diffusion is only supported when options.non_zero_mu_coeff is enabled | ||
""" | ||
if not options.non_zero_mu_coeff: | ||
raise NotImplementedError( | ||
"Heterogeneous diffusion requires options.non_zero_mu_coeff to be enabled" | ||
) | ||
elif not options.heterogeneous_diffusion: | ||
raise NotImplementedError( | ||
"Heterogeneous diffusion requires options.heterogeneous_diffusion to be enabled" | ||
) | ||
|
||
else: | ||
idx = traversals.indexers[traversals.n_dims] | ||
apply_vector = traversals.apply_vector() | ||
|
||
formulae_heterogeneous = tuple( | ||
( | ||
__make_heterogeneous_laplacian( | ||
options.jit_flags, idx.ats[i], options.epsilon, non_unit_g_factor | ||
) | ||
if idx.ats[i] is not None | ||
else None | ||
) | ||
for i in range(MAX_DIM_NUM) | ||
) | ||
|
||
@numba.njit(**options.jit_flags) | ||
def apply(traversals_data, advector, advectee, diffusivity_field): | ||
null_vecfield, null_vecfield_bc = traversals_data.null_vector_field | ||
return apply_vector( | ||
*formulae_heterogeneous, | ||
*advector.field, | ||
*advectee.field, | ||
advectee.bc, | ||
*null_vecfield, | ||
null_vecfield_bc, | ||
*diffusivity_field.field, | ||
diffusivity_field.bc, | ||
traversals_data.buffer, | ||
) | ||
|
||
return apply | ||
|
@@ -62,3 +111,40 @@ def fun(advectee, _, __): | |
) | ||
|
||
return fun | ||
|
||
|
||
def __make_heterogeneous_laplacian(jit_flags, ats, epsilon, non_unit_g_factor): | ||
"""Create heterogeneous Laplacian function that matches MPDATA's one-sided gradient pattern | ||
|
||
Note: Diffusivity values are expected to be non-negative. Negative values will cause | ||
an assertion error. Zero values are handled by setting a minimum threshold (epsilon). | ||
""" | ||
if non_unit_g_factor: | ||
raise NotImplementedError() | ||
|
||
@numba.njit(**jit_flags) | ||
def fun(advectee, _, diffusivity): | ||
# Get concentration values (matching regular laplacian pattern) | ||
c_curr = ats(*advectee, 0) | ||
c_right = ats(*advectee, 1) | ||
|
||
# Get diffusivity values | ||
D_curr = ats(*diffusivity, 0) | ||
D_right = ats(*diffusivity, 1) | ||
|
||
# Input validation for diffusivity values | ||
assert D_curr >= 0.0, "Diffusivity values must be non-negative" | ||
assert D_right >= 0.0, "Diffusivity values must be non-negative" | ||
|
||
# Handle near-zero diffusivity by setting minimum threshold | ||
D_curr = max(D_curr, epsilon) | ||
D_right = max(D_right, epsilon) | ||
|
||
# Match the exact MPDATA pattern but with diffusivity weighting | ||
# Regular: -2 * (c[i+1] - c[i]) / (c[i+1] + c[i] + epsilon) | ||
# Heterogeneous: weight by diffusivity at face | ||
D_face = 0.5 * (D_curr + D_right) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question on the Discretization of Interfacial DiffusivityIn the __make_heterogeneous_laplacian function, the diffusivity at the cell face is calculated using an arithmetic mean. While this is a straightforward approach, for problems involving heterogeneous media, using a harmonic mean for the interfacial diffusivity is often recommended to ensure the continuity of the flux and improve the physical accuracy of the model. Have you considered this alternative? For diffusivity profiles with sharp gradients, the harmonic mean can provide more robust and accurate results. It might be worth comparing both approaches. |
||
|
||
return -2 * D_face * (c_right - c_curr) / (c_right + c_curr + epsilon) | ||
|
||
return fun |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
""" | ||
Comparison between [the *py-pde* example](https://py-pde.readthedocs.io/en/latest/examples_gallery/simple_pdes/pde_heterogeneous_diffusion.html) of d iffusion equation with spatial dependence and the reimplementation with PyMPDATA. | ||
|
||
diffusion_equation_with_spatial_dependence.ipynb: | ||
.. include:: ./diffusion_equation_with_spatial_dependence.ipynb | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current implementation for the heterogeneous Laplacian returns early with
return
whenoptions.non_zero_mu_coeff
isFalse
, but it might be clearer to raise aNotImplementedError
or state in the docstring that heterogeneous diffusion is only supported when this option is enabled.