feat: encountered negative pressure/density cells summary output#3677
feat: encountered negative pressure/density cells summary output#3677
Conversation
…on CPU but doesn't on GPU build + suboptimal
…unt from kernels (optionally)
… logLevel "SolutionDetails"
…ve-pressure-cells
…ve-pressure-cells
paveltomin
left a comment
There was a problem hiding this comment.
looks ok, hopefully no performance impact
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the flow solver's error reporting capabilities by adding detailed logging of element IDs that exhibit negative pressures or densities. The implementation introduces a new component ElementsReporterOutput to manage limited collection and reporting of problematic elements, with configurable output detail level controlled by log levels.
- Implements a buffer-based collection system to track and report negative pressure/density elements
- Replaces simple counter-based negative value reporting with detailed element-specific information
- Adds MPI-distributed table formatting capabilities for consistent multi-rank output
Reviewed Changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| SolutionCheckHelpers.hpp/.cpp | Core implementation of element reporting buffer and output system |
| SolutionCheckKernelsHelpers.hpp | Helper classes for element collection in CUDA kernels |
| SinglePhaseBase.cpp, SinglePhaseWell.cpp, CompositionalMultiphaseWell.cpp, CompositionalMultiphaseFVM.cpp | Integration of new reporting system into existing solvers |
| kernels/singlePhase/SolutionCheckKernel.hpp, kernels/compositional/*.hpp | Kernel updates to use new element collection mechanism |
| TableMpiComponents.hpp/.cpp | New MPI-aware table formatting system |
| TableLayout.hpp/.cpp, TableFormatter.hpp/.cpp, TableData.hpp/.cpp | Table formatting enhancements |
| LogLevelsInfo.hpp, wells/LogLevelsInfo.hpp | Addition of new log level for solution details |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/coreComponents/physicsSolvers/fluidFlow/wells/LogLevelsInfo.hpp
Outdated
Show resolved
Hide resolved
src/coreComponents/physicsSolvers/fluidFlow/kernels/compositional/SolutionCheckKernel.hpp
Outdated
Show resolved
Hide resolved
src/coreComponents/physicsSolvers/fluidFlow/SolutionCheckHelpers.cpp
Outdated
Show resolved
Hide resolved
src/coreComponents/physicsSolvers/fluidFlow/kernels/SolutionCheckKernelsHelpers.hpp
Outdated
Show resolved
Hide resolved
src/coreComponents/physicsSolvers/fluidFlow/SolutionCheckHelpers.hpp
Outdated
Show resolved
Hide resolved
|
@paveltomin I did a benchmark with a real case of 1M+ cells, with the different |
This PR aims at adding a partial logging of the element ids that have negative pressures / densities, to allow the user to quickly see where negative pressure / densities are computed.
To enable the output, it is proposed to set the
logLevelat least to 2 on the flow solver.Here is an exemple of an output (from a synthetic case):
A standard component is proposed to do so this kind of "limited kernel output": the
ElementReporterOutput& . Its goal is to:ElementReporterBuffer, allocate global memory & be able to disable the whole system (ie: by alogLevel),ElementReporterCollector, manage the atomic counting & collection of elements,ElementReporterOutput, output the result (i.e. in the log),If the number of reported elements exceed the rank
ElementReporterBuffersize, the row ends with..., but the elements still gets counted.This table has been drawn by the new
TableTextMpiOutput(also proposed in this PR), which allows to draw a single table on the rank 0 from multiple ranks: