[BUG]: modelchain fills nan with 0

[JoLo90]

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

[JoLo90]

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

[cwhanse]

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.

[markcampanelli]

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?

[JoLo90]

@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 ?