[com1;in3] Change time writing [breaking change]#1219
Merged
Conversation
Contributor
|
Coverage Impact ⬆️ Merging this pull request will increase total coverage on Modified Components (1)
Modified Files with Diff Coverage (2)
🛟 Help
|
awirb
reviewed
Dec 15, 2025
- Export initial timestep (t=0) only if explicitly specified in `tSteps`.
- Updated documentation to reflect changes in timestep export behavior.
- Added tests to ensure correct handling of `tSteps` for both empty and explicit configurations.
- Updated default behavior to export only final timestep when `tSteps` is empty.
1. avaframe/com1DFA/com1DFACfg.ini:16-20
- Changed default from tSteps = 1 to tSteps = (empty)
- Updated comments to reflect new behavior
2. avaframe/in3Utils/fileHandlerUtils.py:387-389
- Removed filtering that excluded t=0 from dtSave array
3. avaframe/com1DFA/com1DFA.py:2097
- Made initial timestep export conditional: only exports if t=0 is in dtSave
4. avaframe/com1DFA/com1DFA.py:2131-2134
- Made Tsave initialization conditional based on whether t=0 is exported
5. docs/moduleCom1DFA.rst:258
- Updated documentation: "initial time step" → "final time step" to reflect new default
… bug timestep saving
- Removed `plotFields` from all configuration files and corresponding test cases.
- Simplified logic to only use `resType` for determining result parameters across all time steps.
- Updated export function to streamline behavior by treating all `resType` fields equally across all time steps.
- Adjusted tests to reflect the removal of `plotFields` and ensure consistency.
Fixed critical bug in timestep saving logic:
- Bug: After saving initial timestep at t=0, dtSave array wasn't updated, causing spurious save at t=dt
- Fix: Added dtSave = updateSavingTimeStep(dtSave, cfgGen, t) after initial save
- This eliminates the extra timestep that was appearing (e.g., t=0, t=0.1, t=10, t=20...)
fso42
force-pushed
the
changeTimeWriting
branch
from
138fe54 to
7a62ff8
Compare
- Removed redundant condition for `resT` by excluding `FTDet` checks. - Simplified logic to populate `resultsDF` unconditionally when appending new rows. fix(com1DFA): correct timestep export condition for t=0 - Updated logic to use `np.any(dtSave <= 1.0e-8)` for handling initial timestep export. - Removed redundant contour fetching logic for dummy fields test(com1DFA): add unit test; squash refactor(tests): fix `test_tSteps_output_behavior` - Updated `com1DFA` to ensure particle directory initialization is handled conditionally during initial export. refactor(com1DFA): ensure valid `resTypes` and improve contour fetching logic - Added logic to append `pfv` to `resTypes` when it only contains invalid or insufficient field types (e.g., `particles` or empty). - Adjusted contour line computation to skip when the target field is a dummy array. refactor(com1DFA): simplify `resTypes` handling and remove redundant `resTypesLast` - Replaced all instances of `resTypesLast` with `resTypes` - Removed unused variable `resTypesLast` across the codebase. - Ensured consistent use of `resTypes` for initializing fields and max value computations.
fso42
force-pushed
the
changeTimeWriting
branch
from
7a62ff8 to
653624a
Compare
awirb
reviewed
Dec 16, 2025
awirb
approved these changes
Dec 16, 2025
…export - Store original `dtSave` to ensure accurate initial timestep decisions. - Update `dtSave` only after initial timestep export to avoid unintended modifications. - Improve readability of conditions by explicitly referencing `dtSaveOriginal`. Root Cause: The code uses dtSave for two decisions but modifies it between them: 1. Line 2107: Checks if dtSave contains t=0 → decides to export initial timestep 2. Line 2114: Calls updateSavingTimeStep() which modifies dtSave (changes [0] to [2*tEnd]) 3. Line 2142: Checks dtSave again → decides whether to add t=0 to Tsave array The Problem: When tSteps = "0": - Line 2107 check passes → exports t=0 - Line 2114 modifies dtSave from [0] to [2*tEnd] - Line 2142 check fails → Tsave remains empty - Result: t=0 was exported but not tracked in Tsave (inconsistency) Proposed Fix: ✓ CORRECT The fix saves the original dtSave before modifications and uses it for both decisions. This ensures consistency between the export and Tsave array decisions. fix(com1DFA): correct field type filter for `resTypes` validation
fso42
force-pushed
the
changeTimeWriting
branch
from
0b563a4 to
84605b4
Compare
Contributor
Author
|
Standardtests ident |
fso42
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Breaking change: by default t=0 is NOT exported anymore
feat(com1DFA): refine tSteps handling for timestep exports
Export initial timestep (t=0) only if explicitly specified in
tSteps.Updated default behavior to export only final timestep when
tStepsis empty.