Skip to content

Address various bugs in physical trough model#1393

Merged
taylorbrown75 merged 18 commits intodevelopfrom
trough_bugs
May 8, 2026
Merged

Address various bugs in physical trough model#1393
taylorbrown75 merged 18 commits intodevelopfrom
trough_bugs

Conversation

@taylorbrown75
Copy link
Copy Markdown
Collaborator

Pull Request Template

Description

Addresses the following bugs in the physical trough model

  • Fix units in end gain test for solar azimuth (compare to pi/2 rather than 90 degrees)
  • Remove duplicate T_startup and T_shutdown var table values
  • Fix trough warning message (labeled as fresnel_physical)
  • Fix units in trough init_fieldgeom() and design_solar_mult()
    • Units are converted in place in init()
    • design_solar_mult() is called before init() so it uses C instead of K
  • Add missing breaks in m_dot_runner switch case
    • Code was always falling to default

Corresponding branches and PRs:

wex, lk, SAM: develop

Unit Test Impact:

SSC unit tests pass. Expect changes to SAM regression tests.

Checklist

  • I've tagged this PR to a milestone

Copy link
Copy Markdown

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

This PR fixes several correctness issues in the physical trough model implementation across the TCS trough receiver and the SSC trough physical compute module, primarily around unit handling, messaging, and control-flow bugs.

Changes:

  • Correct temperature and angle unit handling (avoid double °C→K conversion for HTF properties; compare solar azimuth in radians).
  • Remove duplicate SSC variable table entries and fix an incorrect module name in an exception.
  • Fix unintended switch fallthrough in m_dot_runner() by adding missing break statements.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tcs/csp_solver_trough_collector_receiver.cpp Fixes unit conversions for HTF property calls, corrects solar azimuth comparison units, and prevents switch fallthrough in runner mass flow logic.
ssc/cmod_trough_physical.cpp Removes duplicate var table entries and corrects the module label used in an exec_error.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tcs/csp_solver_trough_collector_receiver.cpp Outdated
@coveralls
Copy link
Copy Markdown

coveralls commented May 5, 2026

Coverage Report for CI Build 25564131132

Coverage decreased (-0.007%) to 56.275%

Details

  • Coverage decreased (-0.007%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • 941 coverage regressions across 3 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

941 previously-covered lines in 3 files lost coverage.

File Lines Losing Coverage Coverage
ssc/tcs/csp_solver_trough_collector_receiver.cpp 563 73.94%
ssc/ssc/cmod_trough_physical_iph.cpp 200 75.34%
ssc/ssc/cmod_trough_physical.cpp 178 75.72%

Coverage Stats

Coverage Status
Relevant Lines: 121270
Covered Lines: 68245
Line Coverage: 56.28%
Coverage Strength: 3451094.77 hits per line

💛 - Coveralls

@taylorbrown75 taylorbrown75 merged commit ed58ac0 into develop May 8, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug csp concentrating solar power

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants