Group species for temporals#591
Conversation
…luxes through patches
There was a problem hiding this comment.
Pull Request Overview
This PR adds functionality to group multiple species together for combined mass flow rate monitoring through temporal diagnostics. Instead of separately tracking species like O2 and N2, users can now define a mixture (e.g., {Air:O2,N2}) to monitor the total combined mass flow rate.
Key changes:
- Extended BPatch to support species groups with new data structures (
groupNames,speciesinGroup,num_groups) - Updated temporal output to sum fluxes across grouped species
- Added documentation and examples for the new grouping syntax
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| Source/PeleLMeX_BPatch.H | Added num_groups field and vectors for storing group names and species-to-group mappings |
| Source/PeleLMeX_BPatch.cpp | Implemented parsing logic for species group syntax {GroupName:Species1,Species2,...} and validation |
| Source/PeleLMeX_Temporals.cpp | Modified output loops to iterate over groups and sum species fluxes for each group |
| Docs/sphinx/manual/LMeXControls.rst | Added documentation and example usage of the species grouping feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| auto it = std::find(names.begin(), names.end(), s); | ||
| if (it != names.end()) { | ||
| size_t index = std::distance(names.begin(), it); | ||
| tmp.push_back(static_cast<int>(index)); |
There was a problem hiding this comment.
Incorrect indexing: speciesinGroup is being filled with mechanism species indices (from std::distance(names.begin(), it)), but it should contain local species indices (0 to num_species-1). In speciesBalancePatch(), these indices are used to access speciesFlux[s], which is indexed by local species indices, not mechanism indices. This will cause incorrect flux summation or potential out-of-bounds access. To fix: find the position of this species in tmp_species_only and use that index instead.
| while (std::getline(ss, item, ',')) { | ||
| auto it = std::find(names.begin(), names.end(), item); | ||
| size_t index = std::distance(names.begin(), it); | ||
| tmp.push_back(static_cast<int>(index)); |
There was a problem hiding this comment.
Incorrect indexing: speciesinGroup is being filled with mechanism species indices (from std::distance(names.begin(), it)), but it should contain local species indices (0 to num_species-1). In speciesBalancePatch(), these indices are used to access speciesFlux[s], which is indexed by local species indices, not mechanism indices. This will cause incorrect flux summation or potential out-of-bounds access. To fix: find the position of each species in tmp_species_only and use that index instead. Also add validation that it != names.end() before computing the distance.
baperry2
left a comment
baperry2
left a comment
There was a problem hiding this comment.
Looks ok to me but Copilot has a lot of suggestions on how to add error checking for the string parsing. Look through those and implement any that seem to have a good value/effort ratio. Let me know when you're done and I'll merge.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
At times, we may need to monitor the combined mass flow rates of a set of species through temporals. For example, we might be interested to monitor the mass flow rate of air through a boundary patch 'inlet'. Currently, the code outputs the MFRs of O2 and N2 separately when the user specifies O2 and N2 in the species list of the bpatch functionality as,
bpatch.inlet.species= O2 N2
This PR lets the user specify a mixture and group species in it in the following way:
bpatch.inlet.species= {Air:O2,N2}
The temporal then outputs the total air MFR under the column 'inlet_Air'.