Skip to content

Comments

Fix time_real used uninitialized in Lagrangian bubble output#1189

Closed
sbryngelson wants to merge 1 commit intoMFlowCode:masterfrom
sbryngelson:fix/time-real-init
Closed

Fix time_real used uninitialized in Lagrangian bubble output#1189
sbryngelson wants to merge 1 commit intoMFlowCode:masterfrom
sbryngelson:fix/time-real-init

Conversation

@sbryngelson
Copy link
Member

@sbryngelson sbryngelson commented Feb 21, 2026

Summary

Severity: HIGH — time column in Lagrangian bubble output contains garbage.

File: src/post_process/m_data_output.fpp, lines 1059 and 1193 (also 1219 in the Silo output variant)

time_real is declared as a local variable but never assigned from file_time (which is read from the restart file and broadcast). It is then written as the time column in the output, producing uninitialized garbage values.

Before

real(wp) :: time_real        ! declared, never assigned
...
call MPI_BCAST(file_time, 1, mpi_p, 0, MPI_COMM_WORLD, ierr)
...
write (29, '(E15.7)') time_real    ! garbage

After

call MPI_BCAST(file_time, 1, mpi_p, 0, MPI_COMM_WORLD, ierr)
time_real = file_time              ! assign from broadcast value
...
write (29, '(E15.7)') time_real    ! correct time

Fixed in both the text output subroutine and the Silo database output subroutine.

Why this went undetected

Lagrangian bubble post-processing is a specialized feature. The other output columns (positions, velocities, radii) are correct, so users may not have noticed the time column being wrong.

Test plan

  • Run Lagrangian bubble simulation with output enabled
  • Verify time column matches expected simulation time

🤖 Generated with Claude Code

Fixes #1209

Copilot AI review requested due to automatic review settings February 21, 2026 03:23
@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 21, 2026

CodeAnt AI is reviewing your PR.


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 21, 2026

Warning

Rate limit exceeded

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

⌛ 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.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@codeant-ai codeant-ai bot added the size:XS This PR changes 0-9 lines, ignoring generated files label Feb 21, 2026
@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 21, 2026

CodeAnt AI finished reviewing your PR.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes uninitialized time_real in Lagrangian bubble output by assigning it from file_time after the MPI broadcast, preventing garbage values in the time column.

Changes:

  • Assign time_real = file_time after MPI_BCAST in two bubble output code paths.

cubic-dev-ai[bot]

This comment was marked as off-topic.

time_real is declared but never assigned from file_time after the
MPI broadcast. The time column in output contains garbage.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov
Copy link

codecov bot commented Feb 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.05%. Comparing base (84c46e0) to head (fa2399b).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1189   +/-   ##
=======================================
  Coverage   44.05%   44.05%           
=======================================
  Files          70       70           
  Lines       20498    20500    +2     
  Branches     1990     1990           
=======================================
+ Hits         9030     9032    +2     
  Misses      10329    10329           
  Partials     1139     1139           

☔ 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.

@sbryngelson
Copy link
Member Author

Superseded by #1240 (batched safe fixes)

@sbryngelson sbryngelson deleted the fix/time-real-init branch February 22, 2026 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 lines, ignoring generated files

Development

Successfully merging this pull request may close these issues.

mv read loop hardcodes 4 instead of nnode

1 participant