Conversation
|
Sorry, but I've missed something. Why don't we use FieldMetric for Bxy, etc? I feel like these should be able to fall back to Field2D. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #645 +/- ##
==========================================
+ Coverage 58.49% 59.77% +1.28%
==========================================
Files 98 98
Lines 10470 10285 -185
Branches 1550 1487 -63
==========================================
+ Hits 6124 6148 +24
+ Misses 3699 3489 -210
- Partials 647 648 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
@bshanahan Good question. Before this I had
but apparently |
This seems like an error in |
Hi @bshanahan If you want to keep the slices then this should assign to Field3DParallel: This is now the way to determine whether expressions should be evaluated in the parallel slices (based on how the output is used, rather than the input type). Changed in this PR: boutproject/BOUT-dev#3430 |
|
Thanks for the clarification @bendudson. How does this affect memory and efficiency? This PR has a few changes from |
This should improve efficiency, especially on GPUs, because expressions involving multiple operations have only one loop over the domain, rather than one loop per operation. We no longer need to store intermediate values so that should improve memory cache use. The main motivation for all this is GPU performance, for which we need to merge operations into kernels. The change from |
|
boutproject/BOUT-dev#3477 introduces |
dschwoerer
left a comment
dschwoerer
left a comment
There was a problem hiding this comment.
Just some minor cleanup.
| } | ||
|
|
||
| // Fci needs communication for the parallel slices | ||
| if (P.isFci()) { |
There was a problem hiding this comment.
Why not
| if (P.isFci()) { | |
| if (P.isFci() or eta_limit_alpha > 0.) { |
and remove above communication?
There was a problem hiding this comment.
Do I care about the applyParallelBoundary here when this is FA? Does it change the physics?
| Coordinates* coord = P.getCoordinates(); | ||
| const Field3D Bxy = coord->Bxy(); | ||
| const Field3D sqrtB = sqrt(Bxy); | ||
| Bxy = coord->Bxy(); | ||
| // If not allocated calculated, otherwise skip as already done | ||
| if (!sqrtB.isAllocated()) { |
There was a problem hiding this comment.
Might as well move everything into the brackets?
There was a problem hiding this comment.
Does the assignment of the field take that much time? Otherwise at some point we have to look through the code and adjust all of these
4 participants
Purpose
This PR adjusts the Braginksii ion and electron viscosity components to be functional in Fci.
Change Summary
Validation
Two 1D slab MMS tests for both viscosity components.
AI Assistance
None
Documentation
None
Review Notes
The asserts are still temporary and I will remove them later. It was just a little bit easier to debug in case something was not working.
Needs boutproject/BOUT-dev#3498 to be finalized.