Skip to content

Create silo series file for ParaView#1381

Open
wilfonba wants to merge 2 commits intoMFlowCode:masterfrom
wilfonba:siloSeriesFile
Open

Create silo series file for ParaView#1381
wilfonba wants to merge 2 commits intoMFlowCode:masterfrom
wilfonba:siloSeriesFile

Conversation

@wilfonba
Copy link
Copy Markdown
Contributor

@wilfonba wilfonba commented Apr 25, 2026

Description

This PR adds tooling for creating a series file in the silo_hdf5/ directory for opening time-series data in newer versions of Paraview. Paraview 6.1.0, which is now compatible with our output files, does not recognize the root/ files as a time-series correctly, which is resolved by creating a series file. The series file is created by listing the files in the silo_hdf5 directory after post-processing has been done, which ensures compatibility with restarts and such.

Fixes #(issue)

Type of change

  • New feature

Testing

How did you test your changes?

Ran a test case and opened the resulting file in Paraview 6.1.0. I also verified backward compatibility with Paraview 5.11.2

AI code reviews

Reviews are not triggered automatically. To request a review, comment on the PR:

  • @coderabbitai review — incremental review (new changes only)
  • @coderabbitai full review — full review from scratch
  • /review — Qodo review
  • /improve — Qodo code suggestions
  • @claude full review — Claude full review (also triggers on PR open/reopen/ready)
  • Add label claude-full-review — Claude full review via label

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 25, 2026

Warning

Rate limit exceeded

@wilfonba has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 58 minutes and 10 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 58 minutes and 10 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b147c34e-f3c0-4695-bb0a-2a8de90ad069

📥 Commits

Reviewing files that changed from the base of the PR and between be1e665 and cc7d36b.

📒 Files selected for processing (3)
  • docs/documentation/visualization.md
  • toolchain/templates/include/generate_silo_series.py
  • toolchain/templates/include/helpers.mako

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions
Copy link
Copy Markdown

Claude Code Review

Head SHA: cc7d36b

Files changed:

  • 3
  • docs/documentation/visualization.md
  • toolchain/templates/include/generate_silo_series.py
  • toolchain/templates/include/helpers.mako

Findings

1. Series file uses integer indices instead of physical simulation time

File: toolchain/templates/include/generate_silo_series.py, line 23

files = [{"name": f"root/{f.name}", "time": i} for i, f in enumerate(collection_files)]

The "time" field in the ParaView .series JSON format is the physical simulation time shown on the time bar when the series is loaded. Using i (0, 1, 2, …) produces a time axis of 0, 1, 2, … regardless of actual simulation time-step sizes or non-uniform output intervals. For a scientific CFD tool this is a correctness problem: time-dependent animations and probes will display wrong time values. The physical time should be extracted from each Silo file (via Silo's Python bindings reading the dtime metadata), or read from a companion file such as time_data.dat if available in the case directory.


2. Filename parsing assumes exactly one underscore in collection_*.silo stem

File: toolchain/templates/include/generate_silo_series.py, line 19

key=lambda p: int(p.stem.split("_")[1]),

split("_")[1] picks the token immediately after the first underscore. If the MFC post-process ever writes files whose stem contains additional underscores (e.g. collection_mpi_0.silo) then split("_")[1] would pick up the wrong token and int() would raise an unhandled ValueError, crashing the script. A more robust pattern is int(p.stem.rsplit("_", 1)[-1]) or a regex match on the trailing numeric suffix.


3. Python script exit code is not checked in the Mako template

File: toolchain/templates/include/helpers.mako, lines 102–104

% if target.name == 'post_process':
    python3 "${MFC_ROOT_DIR}/toolchain/templates/include/generate_silo_series.py" '${os.path.dirname(input)}'
% endif

The generated shell script does not check the return code of this python3 invocation. If the script exits non-zero (e.g. due to a ValueError from the parsing issue above, a permission error writing the series file, or a missing Python installation), the overall mfc.sh run command will still report success, silently producing no .series file. At minimum the invocation should be logged on failure so users can diagnose why no series file was created.

@wilfonba wilfonba changed the title update docs and precheck Create silo series file for ParaView Apr 25, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 25, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.76%. Comparing base (be1e665) to head (cc7d36b).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1381   +/-   ##
=======================================
  Coverage   64.76%   64.76%           
=======================================
  Files          71       71           
  Lines       18713    18713           
  Branches     1549     1549           
=======================================
  Hits        12119    12119           
  Misses       5638     5638           
  Partials      956      956           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant