Skip to content

Fix k_v and k_g module-level variables not initialized to dflt_real#1184

Closed
sbryngelson wants to merge 2 commits into
MFlowCode:masterfrom
sbryngelson:fix/bubble-kv-kg-init
Closed

Fix k_v and k_g module-level variables not initialized to dflt_real#1184
sbryngelson wants to merge 2 commits into
MFlowCode:masterfrom
sbryngelson:fix/bubble-kv-kg-init

Conversation

@sbryngelson

@sbryngelson sbryngelson commented Feb 21, 2026

Copy link
Copy Markdown
Member

Summary

Severity: HIGH — module-level k_v and k_g are left uninitialized.

File: src/simulation/m_global_parameters.fpp, lines 703-704

Every bubble parameter follows the pattern bub_pp%var = dflt_real; var = dflt_real, initializing both the derived-type field and the module-level scalar. But k_v and k_g (thermal conductivities for vapor and gas) skip the module scalar initialization.

Before

bub_pp%k_v = dflt_real;              ! missing: k_v = dflt_real
bub_pp%k_g = dflt_real;              ! missing: k_g = dflt_real
bub_pp%cp_v = dflt_real; cp_v = dflt_real   ! correct pattern

After

bub_pp%k_v = dflt_real; k_v = dflt_real
bub_pp%k_g = dflt_real; k_g = dflt_real

Why this went undetected

Module-level variables may happen to be zero-initialized by the compiler or OS in many environments, masking the missing explicit initialization.

Test plan

  • Run bubble simulation with thermal transport enabled
  • Verify k_v and k_g are correctly set from input

🤖 Generated with Claude Code

Fixes #1204

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

codeant-ai Bot commented Feb 21, 2026

Copy link
Copy Markdown
Contributor

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

coderabbitai Bot commented Feb 21, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@sbryngelson has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 36 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

codeant-ai Bot commented Feb 21, 2026

Copy link
Copy Markdown
Contributor

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Possible Bug
    The PR initializes k_v and k_g at module level but those names are declared as allocatable arrays (real(wp), dimension(:), allocatable). Assigning a scalar (k_v = dflt_real) before allocation will be invalid at runtime (unallocated-array assignment) or will silently do nothing depending on context. Confirm intended type/shape: either these should be scalar module variables or the code should only assign array elements after allocation (or guard with ALLOCATED checks).

  • GPU Sync
    The module declares GPU metadata for bubble parameters (including k_v,k_g) earlier via $:GPU_DECLARE. If k_v/k_g are meant to be visible on the device with default values, ensure the device copy is updated after initialization/after allocation. Otherwise device code may observe uninitialized data.

Comment thread src/simulation/m_global_parameters.fpp Outdated
Comment thread src/simulation/m_global_parameters.fpp Outdated
@codeant-ai

codeant-ai Bot commented Feb 21, 2026

Copy link
Copy Markdown
Contributor

CodeAnt AI finished reviewing your PR.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Initializes module-level thermal conductivity scalars (k_v, k_g) to the default real value to match the initialization pattern used by other bubble parameters and avoid uninitialized module state.

Changes:

  • Initialize module-level k_v alongside bub_pp%k_v.
  • Initialize module-level k_g alongside bub_pp%k_g.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Every other bubble parameter follows the pattern of initializing both
bub_pp%var and the module scalar, but k_v and k_g skip the module
scalar initialization.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
k_v and k_g are allocatable arrays that are only allocated in
s_initialize_bubbles_model when bubbles_euler .and. .not. polytropic.
Assigning a scalar to them in s_assign_default_values_to_user_inputs,
which runs before allocation, is invalid Fortran and causes a runtime
error. Remove the premature array assignments; bub_pp%k_v/k_g (scalars)
are correctly initialized and the arrays are populated from them later
in s_initialize_bubble_vars after allocation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/simulation/m_global_parameters.fpp">

<violation number="1" location="src/simulation/m_global_parameters.fpp:703">
P1: Initialize the module-level scalars `k_v` and `k_g` alongside the derived-type fields to avoid leaving them undefined.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread src/simulation/m_global_parameters.fpp
@codecov

codecov Bot commented Feb 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.05%. Comparing base (84c46e0) to head (af9fcd4).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1184   +/-   ##
=======================================
  Coverage   44.05%   44.05%           
=======================================
  Files          70       70           
  Lines       20498    20498           
  Branches     1990     1990           
=======================================
  Hits         9030     9030           
  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 sbryngelson deleted the fix/bubble-kv-kg-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.

Debug print statement left in post-process production code

2 participants