Skip to content

bug fix with only reading first batch of .trajectory files when plotting multiple dumps#125

Merged
danieljprice merged 2 commits into
mainfrom
trajectory-files
Jun 17, 2026
Merged

bug fix with only reading first batch of .trajectory files when plotting multiple dumps#125
danieljprice merged 2 commits into
mainfrom
trajectory-files

Conversation

@danieljprice

@danieljprice danieljprice commented Jun 17, 2026

Copy link
Copy Markdown
Owner

thanks to Elisa Castro Martínez for reporting...

Summary by CodeRabbit

  • New Features

    • Exact-solution filename inputs now consistently support placeholder substitution across interactive entry, configuration files, and runtime visualization, enabling more flexible file path specification.
  • Improvements

    • Enhanced default file resolution for trajectory and profile configurations with improved fallback handling logic.

…ing multiple dumps (thanks to Elisa Castro Martinez for reporting
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

exact.f90 gains a new resolve_exact_filename subroutine that substitutes %p (rootname up to the last underscore) and %f (full rootname) into filename templates. This helper is then called in the interactive exact-solution filename entry, in read_exactparams fallback storage (which now stores %p-based templates instead of concrete paths), and in exact_solution at runtime before loading exact data.

Changes

Filename placeholder support in exact.f90

Layer / File(s) Summary
resolve_exact_filename subroutine
src/exact.f90
New resolve_exact_filename(filename, rootname) subroutine substitutes %p (rootname up to the last underscore) and %f (full rootname) into template strings; read_exactparams also imports idefaults_file_read for conditional behavior.
Call sites: interactive entry, fallback defaults, and runtime plotting
src/exact.f90
Interactive filename entry in submenu_exact now calls resolve_exact_filename instead of inline %f-only substitution; read_exactparams fallback paths store %p.profile/%p.trajectory templates and disable exact error calculation for trajectory fallbacks; exact_solution resolves filenames before column mapping and data loading.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 Hop, hop, a placeholder's born,
%p and %f replace the thorn,
No more hard-coded paths in sight,
The exact solutions resolve just right,
Templates bloom where filenames grew —
This little bunny cheers for you! 🌸

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title focuses on a specific bug about reading .trajectory files during multi-dump plotting, but the actual changes implement a more comprehensive placeholder resolution system (%f and %p) for exact-solution filenames across multiple contexts (interactive entry, defaults files, and plotting). The title describes one aspect of the fix but does not capture the primary architectural change of introducing the resolve_exact_filename helper and placeholder support. Consider revising to reflect the main technical contribution, e.g., 'Support filename placeholders in exact-solution file handling' or clarify if the .trajectory batch reading was the root cause.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch trajectory-files

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new subroutine resolve_exact_filename to resolve %f and %p placeholders in exact solution filenames, allowing trajectory and profile paths to update dynamically for each dump. The review feedback highlights two key issues: a potential out-of-bounds substring reference in resolve_exact_filename when idash <= 1 which could trigger runtime crashes, and global state pollution caused by modifying the global variable iCalculateExactErrors inside read_exactparams.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/exact.f90
Comment thread src/exact.f90

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/exact.f90`:
- Around line 365-370: Update the print statement's prompt message on line 365
to advertise both the `%f` and `%p` placeholders. Currently the message only
mentions `%f` for the current dump file, but the `resolve_exact_filename`
function now supports `%p` for trajectory/profile representation as well. Modify
the prompt text to document both available placeholder options so interactive
users can discover and utilize the new `%p` functionality.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5170f8f1-719d-4679-9ad3-5774c06b27b7

📥 Commits

Reviewing files that changed from the base of the PR and between 876f0fa and 83d1395.

📒 Files selected for processing (1)
  • src/exact.f90

Comment thread src/exact.f90 Outdated
@danieljprice
danieljprice merged commit 0cf7764 into main Jun 17, 2026
5 checks passed
@danieljprice
danieljprice deleted the trajectory-files branch June 17, 2026 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant