[BUG]: modelchain fills nan with 0

Describe the bug

We fillna(0) twice in modelchain.py

pvlib-python/pvlib/modelchain.py

Line 769 in 9a0ad55

self.results.ac = ac.fillna(0)

pvlib-python/pvlib/modelchain.py

Line 668 in 9a0ad55

self.results.dc = tuple(dc.fillna(0) for dc in self.results.dc)

I can open a PR deleting these lines, which would make more sense to me, unless there was a specific reason to do so ?

To Reproduce

def calculate_pv_system_production(
    pv_system: SimplifiedPVSystem, solar_inputs: SolarInputs
) -> np.ndarray:
    """
    Returns AC power output in kW using the PVWatts model.
    """
    # 1. Location
    location = pvlib.location.Location(
        latitude=pv_system.coordinates.latitude,
        longitude=pv_system.coordinates.longitude,
        tz=str(pv_system.timezone),
        altitude=pv_system.coordinates.altitude,
    )

    # 2. System
    system = pvlib.pvsystem.PVSystem(  # todo review all of these parameters with dict
        surface_tilt=pv_system.surface_tilt,
        surface_azimuth=pv_system.surface_azimuth,
        module_parameters={
            "pdc0": pv_system.solar_power_kwp * 1000,  # W peak DC
            "gamma_pdc": pv_system.temp_coefficient,
        },
        inverter_parameters={
            "pdc0": pv_system.solar_power_kwp * 1000,
            "eta_inv_nom": pv_system.inverter_efficiency,
        },
        temperature_model_parameters=pvlib.temperature.TEMPERATURE_MODEL_PARAMETERS[
            "sapm"
        ]["open_rack_glass_glass"],
    )

    # 3. ModelChain
    mc = pvlib.modelchain.ModelChain(
        system,
        location,
        dc_model="pvwatts",
        ac_model="pvwatts",
        aoi_model="physical",
        spectral_model="no_loss",
        losses_model="pvwatts",
    )

    # 4. Run model — pvlib computes sun position internally from location + times
    weather_data = {
        "dni": solar_inputs.dni_wm2,
        "dhi": solar_inputs.dhi_wm2,
        "ghi": solar_inputs.ghi_wm2,
    }
    if solar_inputs.outdoor_air_temp_c is not None:
        weather_data["temp_air"] = solar_inputs.outdoor_air_temp_c

    solar_inputs_pvlib_format = pd.DataFrame(
        index=solar_inputs.ts,
        data=weather_data,
    )

    mc.run_model(solar_inputs_pvlib_format)

    # 5. Return AC power in kW
    return (mc.results.ac / 1000).to_numpy()

Expected behavior
Don't fill NaNs with 0, keep NaNs

Screenshots
This

Becomes

Versions:

pvlib.__version__: 0.15.2
pandas.__version__:
python: 3.10

Humm just saw a PR was opened yesterday #2822, and if I would fix it would pretty much look alike

@JoLo90

Overlaps with #1409

@JoLo90 you are already ahead of the game by starting a discussion :)

I suspect that the "set to 0" statements were added to enable lazy summation of energy. Maybe we can add those sums to ModelChainOutput.

My other concern with just removing the set to 0 lines is this: NaNs can be produced by models along the model chain, even from valid weather input. If we remove the set to 0 statements (and I think we should) we may introduce additional NaNs into the output that don't reflect NaN input. Should we distinguish between weather NaN and other-source NaN? I don't know. My reaction is "ugh", because I'm imagining some kind of weather NaN filter being applied to AC output, which may make it inconsistent with the DC stage, etc.

For reference, there is numpy.nansum. As a skeptical user, I'd prefer to "opt in" to any usage of this (perhaps through clear disclosure in a docstring). At the cost of interface and implementation complexity, it would be nice to be made aware when such a "workaround" was needed on my particular simulation.

From the discussion in #2808, it sounded like "softer" model failures, such as Perez producing a negative total sky diffuse irradiance on the POA, should be handled differently from merely producing NaN, even though this might signify just as much a "garbage in" situation as a direct NaN input.

Because a pvlib model run can "pass through" or "fail silently" (e.g., not raise on a NaN or otherwise detected bad value), I think at the very least some guidance is warranted in the docs on how users should interpret NaNs (which are fairly easy to work around) vs. the onus to check for other (possibly more subtle) non-sensical results. For example, if a negative POA is buried deep in a ModelChain computation, then is it even detectable in the final energy result? Is it being suggested here that ModelChain play the role of the "savvy user" and perform this layer of additional checking?

@markcampanelli

@markcampanelli @cwhanse
In the example I showed, I would have expected np.nan because, at least to me, nan means "We don't know" while 0 means "There is no pv production".
@markcampanelli can you show me an example where the Perez model "produce a negative total sky diffuse irradiance on the POA" ? Or some other examples where we rather pass NaN instead of negative values ?

@JoLo90

@JoLo90 Here are morning and afternoon examples I generated from an NSRDB TMY file, which had negative horizon diffuse in 516 of 8760 timestamps. (I think this phenomenon is called horizon darkening.) My understanding is that this is "correct" Perez model behavior. However, one can also choose inputs that produce very questionable negative diffuse components (and not just in the horizon).

('Location: Location: \n  name: Barstow, CA, USA\n  latitude: 34.89\n  longitude: -117.02\n  altitude: 726.0\n  tz: Etc/GMT+8',
 Timestamp('1999-01-01 09:30:00-0800', tz='Etc/GMT+8'),
 'solar_zenith: 66.91816055106597',
 'solar_azimuth: 144.57772678693345',
 'DHI: 239.0',
 'DNI: 68.0',
 'poa_circumsolar: 93.92630303181842',
 'poa_isotropic: 174.63442916267516',
 'poa_horizon: -2.007727112695456',
 'poa_sky_diffuse (total): 266.5530050817982')

('Location: Location: \n  name: Barstow, CA, USA\n  latitude: 34.89\n  longitude: -117.02\n  altitude: 726.0\n  tz: Etc/GMT+8',
 Timestamp('1999-01-06 14:30:00-0800', tz='Etc/GMT+8'),
 'solar_zenith: 68.25788085275711',
 'solar_azimuth: 218.77922984602714',
 'DHI: 173.0',
 'DNI: 37.0',
 'poa_circumsolar: 48.51667744898173',
 'poa_isotropic: 138.5259789964514',
 'poa_horizon: -1.8861462897438006',
 'poa_sky_diffuse (total): 185.15651015568932')

UPDATE: I forgot to include that surface_tilt = 20.0 and surface_azimuth = 180.0.

@markcampanelli

@markcampanelli thanks for the example.
I would say that as long as it makes mathematical and physical sense (if pow_horizon is negative because it lowers the poa_sky_diffuse and thus makes its calculation more precise) we could keep negative values.
I think it is not problematic as long as it's not the the "final" output (ex: negative DC/AC power doesn't make sense, if it is due to numerical instability clip to 0 would be my approach). In a way, we decide for the user what the correct value is, and we don't do GIGO anymore, but I think this is domain knowledge of the pvlib maintainers and the added value of the pvlib library. The problem with this issue is that we don't respect the nan in -> nan out philosophy anymore.

@cwhanse

Hello @cwhanse @JoLo90
Since #2822 was closed pending agreement here, I removed both fillna(0) calls on main to see where NaN shows up from valid weather, which was @cwhanse's concern. I found four cases:

1. Trackers at night: SingleAxisTrackerMount.get_orientation returns NaN tilt when the sun is down, so every night hour becomes NaN for every DC model (4358 h in the bundled TMY3). ModelChain used to stow at axis_tilt in _prep_inputs_tracking, but that hasn't been called since #1771. If I put the stow back, the NaNs go away and nothing else changes.
2. spectral_model='first_solar' at night: Airmass is NaN for zenith > 90, so the modifier is NaN even when POA is 0. Checking for poa == 0 isn't enough: 297 TMY3 hours just after sunset still have POA > 0 because of hourly averaging. Using a modifier of 0 where airmass is NaN, as spectral_factor_sapm already does, fixes it, and annual energy is the same as on main.
3. Negative night irradiance: 210 rows in the SURFRAD test file give negative I_L and R_sh, and cec/desoto/pvsyst return NaN.
4. Tiny positive irradiance (#1673), e.g. E = 1e-15 W/m² with cec.

Both 3 and 4 go away if effective irradiance below 1e-9 is set to 0 before calcparams, as suggested in #1673. NaN still passes through. With that change and the first_solar fix, I compared 540 outputs against main (TMY3 and SURFRAD,ccec/desoto/pvsyst, all AOI and spectral models, single and multi-array). Every value that is valid in both matches, apart from one lambertw case mentioned at the end. The existing modelchain tests also pass without edits.

A few questions before I open a PR:

Trackers: wait for Consider updating pvlib.tracking.singleaxis() to return zeros (or similar) for nighttime instead of NaNs #2539 (default night stow), or restore the stow inside ModelChain for now?
Should the first_solar fix go in ModelChain or in spectral_factor_firstsolar?
Is the 1e-9 clip acceptable?
Energy sums in ModelChainOutput: in this PR or a follow-up?

Separately, singlediode can still return NaN when v_oc comes out around 1e-26 instead of exactly 0. #2862 doesn't cover that, so I'll open a separate issue for it.

Happy to get assigned and open the PR once we agree on a direction

@varma1221

@varma1221 thanks for following up

1. Trackers at night: I am wondering if it wouldn't be easier to avoid filtering for sun above panel horizon and simply pass the returned values at night. Removing the checks returns a vertical surface at night see below.

Same here maybe passing the max airmass at horizon 38 (based on https://en.wikipedia.org/wiki/Air_mass_(astronomy)) would be easier, but I still haven't check if it would break or produce unrealistic calculations results downwards.

Navigation Menu

Repository navigation

[BUG]: modelchain fills nan with 0 #2823

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[BUG]: modelchain fills nan with 0 #2823

Description

Activity

JoLo90 commented on Jul 14, 2026

cwhanse commented on Jul 14, 2026

markcampanelli commented on Jul 14, 2026

JoLo90 commented on Jul 14, 2026

markcampanelli commented on Jul 15, 2026

JoLo90 commented on Jul 21, 2026

varma1221 commented on Oct 3, 2026

JoLo90 commented on Oct 6, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions