Sanity checks on albedo values?

[sixyphe]

Hi team! I've tracked down my last two bugs, I'd like to discuss one of them. My issue was that the test TMY weather data I retrieved from PVGIS had an albedo data column (which is good), but since there was no data, all cells in that column were filled with a "999" value. Since, as of 0.9.2, pvlib.modelchain.ModelChain.run_model() now takes albedo values from the weather data into account, you can see how that can be a problem...
See attached code for a demonstration (my environment: python 3.10.11, numpy 1.25.0, pandas 2.0.3)
I don't consider this to be a bug per se; I believe it's up to the user to sanitize their data. But since albedo is expected to be between 0 and 1, it might be nice to have a check that raises a specific warning or error and/or replaces those values with a set value (0, 0.3, something like that).
Also, it looks like an albedo parameter provided explicitely in the array definition is ignored when albedo values are present in the weather data. Is that the expected behaviour? Is that the desirable behaviour?
If there is a community consensus for a course of action, I'll be happy to submit a formal code suggestion.
Thanks!

sixyphe_albedo_example.py.txt

[sixyphe]

The code, if you don't want to bother with the download:

# -*- coding: utf-8 -*-
"""
Created on Fri Jul  7 18:07:33 2023

@author: sixyp
"""

import pvlib

# Set system location
test_location = pvlib.location.Location(36, 10)

# Import a weather file built by PVGIS according to latitude and longitude
weather_data_with_albedo, _, _, _ = pvlib.iotools.get_pvgis_tmy(
    map_variables=True,
    latitude=test_location.latitude,
    longitude=test_location.longitude,
    outputformat='epw')

# Example albedo value
print("Random albedo value from the weather data: ",
      weather_data_with_albedo.albedo[0])

# Create a version of the weather data without the albedo column
weather_data_without_albedo = weather_data_with_albedo.drop("albedo", axis=1)

# Set an albedo - this value will be ignored when data is present
test_albedo = 0.3

# Get the module from the SAM database
test_module_parameters = pvlib.pvsystem.retrieve_sam('cecmod')[
    'Canadian_Solar_Inc__CS5C_80M']

# Build the array
test_array = pvlib.pvsystem.Array(
            mount=pvlib.pvsystem.FixedMount(
                50, 180, 'open_rack'),
            albedo=test_albedo,
            module_parameters=test_module_parameters,
            module_type='glass_polymer',
            modules_per_string=5,
            strings=1,
            )

# Build the system
test_system = pvlib.pvsystem.PVSystem(
            arrays=[test_array],
            inverter_parameters=dict(pdc0=3)
            )

# Build the ModelChain
test_mc = pvlib.modelchain.ModelChain(
            system=test_system,
            location=test_location,
            aoi_model='physical',
            spectral_model='no_loss')

print("****Starting model without albedo****")
test_mc.run_model(weather_data_without_albedo)
print("****Finished model without albedo****")

print("****Starting model with albedo****")
test_mc.run_model(weather_data_with_albedo)
print("****Finished running model with albedo****")

[kandersolar]

I don't consider this to be a bug per se; I believe it's up to the user to sanitize their data. But since albedo is expected to be between 0 and 1, it might be nice to have a check that raises a specific warning or error and/or replaces those values with a set value (0, 0.3, something like that).

A bit of context: the original MATLAB version included these kinds of range checks (example), but we have avoided them in pvlib-python. The responsibility is on the user to provide clean inputs, as you say.

Also, it looks like an albedo parameter provided explicitely in the array definition is ignored when albedo values are present in the weather data. Is that the expected behaviour? Is that the desirable behaviour?

Expected: yes, see #1387 and #1478. Desirable? Perhaps up to the user ;)

[kandersolar]

@AdamRJensen, should we update the PVGIS function to replace 999 with nan?

[AdamRJensen]

@AdamRJensen, should we update the PVGIS function to replace 999 with nan?

Yes, certainly! I could do that if nobody else volunteers 😀

[sixyphe]

I'm about to leave on vacation for a few weeks (whee!), but I'd be happy to suggest a code change when I'm back, if there's no rush. Thanks all!

[AdamRJensen]

I'm about to leave on vacation for a few weeks (whee!), but I'd be happy to suggest a code change when I'm back, if there's no rush. Thanks all!

@sixyphe please do so, much appreciated!