Fix upwind/downwind finite volume operators#3979
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
…o issue-3526-upwind
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3979 +/- ##
===========================================
- Coverage 99.58% 99.58% -0.01%
===========================================
Files 257 257
Lines 21254 21248 -6
===========================================
- Hits 21166 21160 -6
Misses 88 88 ☔ View full report in Codecov by Sentry. |
valentinsulzer
left a comment
There was a problem hiding this comment.
Thanks, looks good. Can you show plots of the upwinding example vs analytical solution before and after the fix?
tests/unit/test_spatial_methods/test_finite_volume/test_finite_volume.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>
|
Thanks, that does look much better! Is the mismatch with the analytical solution because it's not a high-enough order scheme? |
|
I think so, as refining the mesh gives a better result. I need to do some more reading on this, and potentially implement higher order methods, but that's a separate issue. |
* pybamm-team#3526 initialise branch * pybamm-team#3256 add UpwindDivergence and DownwindDivergence * work in progress for fixing upwind scheme * pybamm-team#3526 add missing argument docstring * pybamm-team#3526 fix upwind/downwind methods * style: pre-commit fixes * pybamm-team#3526 get finite volume notebook from develop * 3526 add temporary test * ruff * style: pre-commit fixes * pybamm-team#3526 remove unused variable * pybamm-team#3526 clarify wording * pybamm-team#3526 fix tests * pybamm-team#3526 add to CHANGELOG * pybamm-team#3526 update coverage * pybamm-team#3526 add integration test * style: pre-commit fixes * pybamm-team#3526 remove example for debugging * Remove print for debug Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com> * pybamm-team#3526 add second test * pybamm-team#3526 generalised example but did not add test as was hard to know what to measure * style: pre-commit fixes --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com> Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com>






Description
Fixed the issue with the upwind and downwind operators. I am now leveraging the
add_ghost_nodesmethod, to which I had to do some minor changes.Fixes #3526
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.
Key checklist:
$ pre-commit run(or$ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)$ python run-tests.py --all(or$ nox -s tests)$ python run-tests.py --doctest(or$ nox -s doctests)You can run integration tests, unit tests, and doctests together at once, using
$ python run-tests.py --quick(or$ nox -s quick).Further checks: