Pressure solvers for everything! #589
Merged
Conversation
This file contains 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
This reverts commit e109275.
ali-ramadhan
added
feature 🌟
ali-ramadhan
added
the
numerics 🧮
This was referenced Jan 8, 2020
ali-ramadhan
changed the title
Codecov Report
@@ Coverage Diff @@
## master #589 +/- ##
==========================================
+ Coverage 71.64% 76.42% +4.78%
==========================================
Files 77 124 +47
Lines 2095 2117 +22
==========================================
+ Hits 1501 1618 +117
+ Misses 594 499 -95
Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR is a stab at implementing the pressure solvers plan/overhaul outlined in #586. So far it implements pressure solvers for all types of domains (triply/doubly periodic, channels, and boxes) on regular grids (except boxes on GPUs for now).
Equally important, it refactors and cleans up the pressure solvers so that we can now easily add in pressure solvers for vertically stretched grids and distributed models.
A lot of cleanup was done in the process as well. Pressure solver code and kernels have been fully moved from the
TimeStepperssubmodule to theSolverssubmodule. Different parts have been reorganized into multiple files and the index permutation business has been abstracted away a bit so we have fewer ugly kernels (and fewer kernels!).I could have gone much further in condensing the code base and number of pressure solvers. You'll notice quite a bit of code repetition, but I think for now and the forseeable future this is actually a good thing. The pressure solver code is nontrivial and keeping it as simple as possible will improve the readability and longevity of the code base.
It's also easier to add and modify pressure solvers now. For example, if @sandreza wants to implement a fast pressure solve that only works for horizontally periodic domains (Kleiser-Schumann?), it's now much easier to do that.
I should probably add some benchmarks.