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

Added Forward, Backward and Central Difference Option. #1

Merged
merged 5 commits into from
Mar 20, 2023

Conversation

jejjohnson
Copy link
Contributor

TLDR: I added a forward, central, and backward difference option for all of the finite difference methods.


Problem:

There are many cases in PDEs where we need to use the backward difference method. For example, any advection PDE requires a backwards finite difference scheme otherwise the solution blows up (e.g. 12 steps NS). Currently in the package, the package only implements the central difference except in the special case where the central difference stencil was too small for the input size). However, the package is able to generate the offsets for all 3 schemes. I think having the option to choose which finite difference scheme would be extremely advantageous for all users. I myself needed it write away for some of the code I am using (e.g. 1d linear convection)


Solution:

  • I added the forward and the backward difference scheme to the difference function which is callable via a method="central"|"forward"|"backward". I also added this option to all of the subsequent functions that depend on the difference function, i.e. the rest of them.
  • I also updated all of the tests to reflect the changes. They pass. I only had to change the tolerance for one of the tests.
  • It is backwards compatible with the previous version because the default ("central") is still the default for all methods.
  • I fixed a few typos here and there.

Potential Improvements

  • I didn't see a place to put this into the fgrad function. This might be helpful for some people in the future.
  • I did not add an option to have different schemes for different dimensions, e.g. grad, divergence, etc. I can see this being potentially useful in the future.

otherwise it wont install without manually adding jaxlib.
- central, backward, forward
Added to difference function
Added to all functions that depend on difference.
- specific tests for each difference method
- parameterized tests for all of subsequent methods
- increased tolerance for laplacian for forward.
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@ASEM000
Copy link
Owner

ASEM000 commented Mar 17, 2023

Hello, thank you for your contribution.

I will try to merge As soon as possible.

p.s. for fgrad, you can use offests = [-1,0] to define the backward difference.
I will try to improve the documentation of fgrad to reflect this piece of info.

@codecov
Copy link

codecov bot commented Mar 20, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 🎉

Comparison is base (5ebbe3b) 99.75% compared to head (e817320) 99.78%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main       #1      +/-   ##
==========================================
+ Coverage   99.75%   99.78%   +0.02%     
==========================================
  Files           6        6              
  Lines         416      468      +52     
==========================================
+ Hits          415      467      +52     
  Misses          1        1              
Impacted Files Coverage Δ
finitediffx/_src/finite_diff.py 100.00% <100.00%> (ø)
tests/test_finite_diff.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ASEM000 ASEM000 merged commit b60ec9a into ASEM000:main Mar 20, 2023
@ASEM000
Copy link
Owner

ASEM000 commented Mar 20, 2023

@jejjohnson Great work 🎉

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

Successfully merging this pull request may close these issues.

2 participants