Make volume integrals, indicators, etc dimension agnostic#2488
Make volume integrals, indicators, etc dimension agnostic#2488ranocha merged 46 commits intotrixi-framework:mainfrom
Conversation
Review checklistThis checklist is meant to assist creators of PRs (to let them know what reviewers will typically look for) and reviewers (to guide them in a structured review process). Items do not need to be checked explicitly for a PR to be eligible for merging. Purpose and scope
Code quality
Documentation
Testing
Performance
Verification
Created with ❤️ by the Trixi.jl community. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2488 +/- ##
==========================================
+ Coverage 96.79% 96.80% +0.01%
==========================================
Files 525 527 +2
Lines 42708 42611 -97
==========================================
- Hits 41337 41248 -89
+ Misses 1371 1363 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
If we want this (and I am in favor of it), I would propose to put the dimension agnostic functions into dimension-general files, e.g., moving the |
That's an excellent idea! |
…ixi.jl into CleanUpVolumeIntegrals
|
Following a similar idea, wouldn't it also make sense to put mesh-agnostic functions (at least the ones, which are the same of all mesh types) into mesh-general files like https://github.com/trixi-framework/Trixi.jl/blob/531ee91d0dbe93e8fb8bf9f675b34f7982b9077e/src/solvers/dg.jl. Currently, we have quite some functions, which also allow non- |
I like the idea and I would be fine with adding a file like Lines 830 to 837 in 531ee91 |
|
Yes, it would be against this code structure, but the question is, which code structure is easier to understand and work with. I was just last week talking to a Trixi.jl newcomer, who wants to contribute to Trixi.jl and was confused about seeing code relevant for the |
…hring/Trixi.jl into CleanUpVolumeIntegrals
ranocha
left a comment
There was a problem hiding this comment.
Could you please move the comment explaining the flux differencing strategy for curved meshes to an appropriate location and ping me for approval of this PR afterward?
* Make volume integrals dimension agnostic * clean up indicators * bring back dim spec version * bring back indicators * Apply suggestions from code review * remove create cahce * remove 3d * move functions * restructure * comment * comments * rename * fmt * comment * move * test vals * specify solver * fmt * comments --------- Co-authored-by: Hendrik Ranocha <ranocha@users.noreply.github.com>
…ework#2488) * Make volume integrals dimension agnostic * clean up indicators * bring back dim spec version * bring back indicators * Apply suggestions from code review * remove create cahce * remove 3d * move functions * restructure * comment * comments * rename * fmt * comment * move * test vals * specify solver * fmt * comments --------- Co-authored-by: Hendrik Ranocha <ranocha@users.noreply.github.com>
Probably due to historical debt, the volume integrals have been implemented for every dimension. There is currently no reason to do so, as this results only in duplicated code and makes changes such as discussed in #2472 and #2149 unnecessary cumbersome.
Holds also true for some other functions (mostly
create_cache)