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

compiler: Patch symbolic coefficients over cross derivatives #2248

Merged
merged 5 commits into from
Oct 27, 2023

Conversation

FabioLuporini
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Merging #2248 (a30b392) into master (4b0e39a) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2248      +/-   ##
==========================================
+ Coverage   86.88%   86.90%   +0.02%     
==========================================
  Files         229      229              
  Lines       41691    41759      +68     
  Branches     7696     7703       +7     
==========================================
+ Hits        36223    36292      +69     
- Misses       4839     4840       +1     
+ Partials      629      627       -2     
Files Coverage Δ
devito/finite_differences/coefficients.py 93.83% <100.00%> (+0.70%) ⬆️
devito/finite_differences/differentiable.py 94.12% <100.00%> (+0.21%) ⬆️
devito/types/dense.py 93.88% <ø> (-0.04%) ⬇️
tests/test_symbolic_coefficients.py 99.28% <100.00%> (+0.15%) ⬆️
tests/test_unexpansion.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@@ -22,7 +22,7 @@ def test_backward_dt2(self):
assert_structure(op, ['t,x,y'], 't,x,y')


class TestSymbolicCoefficients(object):
class TestSymbolicCoeffs(object):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think we should also test the case where different coefficients are passed for x and y derivatives with cross-derivatives, mixed derivatives, and non-perfect mixed derivatives. Also the case where only one of the x or y derivatives gets coefficients specified but bother are defined as symbolic.

Operator(Eq(u, u.dx.dx, coefficients=coeffs), opt=opt).cfunction

# Non-perfect mixed derivative
Operator(Eq(u, (u.dx + v.dx).dx, coefficients=coeffs), opt=opt).cfunction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to sanity check these outputs. Print them and you will see that no errors are raised currently, but the resultant stencils are incorrect

Copy link
Contributor

@EdCaunt EdCaunt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, should tide things over

@FabioLuporini FabioLuporini merged commit 7a235ed into master Oct 27, 2023
31 of 32 checks passed
@FabioLuporini FabioLuporini deleted the patch-unexp-cross-derivs branch October 27, 2023 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants