Skip to content

Conversation

@Chirag3841
Copy link

@Chirag3841 Chirag3841 commented Dec 6, 2025

@cwhanse

The problem was that 'noct_sam' did not correctly account for the
'effective_irradiance' parameter. The original implementation always used
'poa_global' when computing the initial temperature rise, which caused:

  • Open-circuit modules (efficiency = 0) to ignore 'effective_irradiance'
  • Very low 'effective_irradiance' values to produce unrealistic cooling
  • Results inconsistent with SAM under certain option combinations

This PR updates 'noct_sam' so that:

  • 'effective_irradiance' is used consistently when provided
  • Negative or very small irradiance values are clipped to zero
  • Computation now matches SAM’s documented behaviour
  • I updated the noct_sam function to correctly incorporate effective irradiance into the temperature model.

New tests were added to validate:

  • Temperature decreases with reduced effective irradiance
  • Open-circuit modules still respond to effective irradiance changes
  • Temperature never falls below ambient for extremely small irradiance

These tests pass under the main branch, confirming correct behaviour.

@Chirag3841
Copy link
Author

@cwhanse
Kindly review the updated implementation.

@Chirag3841 Chirag3841 changed the title Fix noct effective irradiance Fix noct_sam() effective irradiance Dec 6, 2025
@Chirag3841 Chirag3841 changed the title Fix noct_sam() effective irradiance Fix noct_sam() Dec 6, 2025
@cwhanse cwhanse added the bug label Dec 7, 2025
@cwhanse
Copy link
Member

cwhanse commented Dec 7, 2025

@Chirag3841 please merge the current main, this PR is missing the changes made in #2605

@cwhanse
Copy link
Member

cwhanse commented Dec 7, 2025

I don't think we want to patch noct_sam this way. My thought is that we replace poa_global with effective_irradiance in the first arg position, drop effective_irradiance=None, and do away with the POA ratio internally. That matches the SAM calculation. The SAM function also handles calculation of effective irradiance, which isn't the job of this function in pvlib.

Replacing the argument requires a deprecation. @Chirag3841 if the deprecation machinery is a bit much, OK to close this PR and thanks for attempting the fix.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NOCT_SAM may not handle effective irradiance correctly

2 participants