-
Notifications
You must be signed in to change notification settings - Fork 143
(feat) UTVD fluxlimiter #2374
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
(feat) UTVD fluxlimiter #2374
Conversation
866076a to
1240d20
Compare
1240d20 to
dfeb932
Compare
# Conflicts: # make/makefile # msvs/mf6core.vfproj # src/Model/TransportModel/InterpolationScheme/AdvSchemeEnum.f90 # src/Model/TransportModel/InterpolationScheme/CentralDifferenceScheme.f90 # src/Model/TransportModel/InterpolationScheme/TVDScheme.f90 # src/Model/TransportModel/tsp-adv.f90 # src/Utilities/LinearAlgebraUtils.f90 # src/meson.build
42b2e65 to
1306626
Compare
aprovost-usgs
left a comment
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.
Elegant code. Just minor things for your consideration
…d of the G matrix
# Conflicts: # make/makefile
# Conflicts: # make/makefile # src/Model/TransportModel/tsp-adv.f90
mjr-deltares
left a comment
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.
I really like how you did this. I do think we need to present the core design to the team so if they visit tsp-adv, they know what is going on there. You have increased the level of software engineering to come up with a better structured code, and succeeded. Let's make sure people can follow along.
|
|
||
| select case (this%limiter_id) | ||
| case (2) ! van Leer | ||
| theta = max(0.0_dp, min((r + dabs(r)) / (1.0_dp + dabs(r)), 2.0_dp)) |
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.
I know we have DZERO, DONE, etc. but I like it better how you have done it here, it's readable
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.
Nice to isolate generic functionality. There are similar units in the ModelUtilities folder. Should this go there as well?
aprovost-usgs
left a comment
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.
Nice job, @Manangka
# Conflicts: # make/makefile
This commit adds the new TVD flux limiter based on the paper of Darwish.
For now i called the new limiter UTVD where U stands for unstructured. But as @christianlangevin and i discussed last week this doesn't cover the because it is also suited for structured grids.
A small description of the code layout.
The main starting point is the tsp-adv.f90 file. Here the cell based gradient is created and passed on to the UTVD class.
I would recommend reviewing tsp-adv.f90 -> IGradient.f90/LeastSquaredGradient.f90 -> UTVDScheme.f90 first to follow the logic of the implementation.
A helper class
DisUtilsis added which provided common functions e.g. getting a cell centroid locationI've updated existing advection tests to also test the new limiter.
It depends on #2367 and #2370
Checklist of items for pull request
ruffon new and modified python scripts in .doc, autotests, doc, distribution, pymake, and utils subdirectories.fprettifyFor additional information see instructions for contributing and instructions for developing.