Skip to content

Expose first_solar_spectral_correction #466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
May 31, 2018
Merged

Expose first_solar_spectral_correction #466

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
ffd685c
Add first_solar_spectral_correction method, improve infer_spectral_model
cwhanse May 23, 2018
0881f01
edits: input testing, first_solar_spectral_coefficients name
cwhanse May 25, 2018
6d47188
rename first_solar_spectral_correction method to first_solar_spectral…
cwhanse May 25, 2018
7a04b02
add test for pvsystem.first_solar_spectral_loss
cwhanse May 25, 2018
88fe65e
Fix declarations of weather and airmass in test_spectral_models
cwhanse May 26, 2018
2da94e3
Correct errors and expected value in test_spectral_models
cwhanse May 28, 2018
210e813
Fix space error
cwhanse May 28, 2018
dec5d95
Fix handling of weather in test_spectral_models
cwhanse May 28, 2018
0732b4c
Change test_PVSystem_first_solar_spectral_loss and test_spectral_models
cwhanse May 29, 2018
ce78670
Fix error in first_solar_spectral_loss
cwhanse May 29, 2018
e8de2e0
Formatting and flake8
cwhanse May 30, 2018
bd7e119
Add constant_spectral_loss back to test_spectral_models arguments
cwhanse May 30, 2018
19de627
Fix line continuation in pvsystem.py
cwhanse May 30, 2018
67a4561
whatsnew, docstring for pvsystem.first_solar_spectral_loss, remove no…
cwhanse May 30, 2018
e4724ae
Fix typo in pvsystem.py
cwhanse May 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions pvlib/atmosphere.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,6 @@ def gueymard94_pw(temp_air, relative_humidity):

return pw


def first_solar_spectral_correction(pw, airmass_absolute, module_type=None,
coefficients=None):
r"""
Expand Down Expand Up @@ -466,8 +465,11 @@ def first_solar_spectral_correction(pw, airmass_absolute, module_type=None,
coefficients = _coefficients[module_type.lower()]
elif module_type is None and coefficients is not None:
pass
elif module_type is None and coefficients is None:
raise TypeError('No valid input provided, both module_type and ' +
'coefficients are None')
else:
raise TypeError('ambiguous input, must supply only 1 of ' +
raise TypeError('Cannot resolve input, must supply only one of ' +
'module_type and coefficients')

# Evaluate Spectral Shift
Expand Down
21 changes: 16 additions & 5 deletions pvlib/modelchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ def sapm_aoi_loss(self):
return self

def no_aoi_loss(self):
self.aoi_modifier = 1
self.aoi_modifier = 1.0
return self

@property
Expand All @@ -532,7 +532,7 @@ def spectral_model(self, model):
elif isinstance(model, str):
model = model.lower()
if model == 'first_solar':
raise NotImplementedError
self._spectral_model = self.first_solar_spectral_loss
elif model == 'sapm':
self._spectral_model = self.sapm_spectral_loss
elif model == 'no_loss':
Expand All @@ -546,13 +546,24 @@ def infer_spectral_model(self):
params = set(self.system.module_parameters.keys())
if set(['A4', 'A3', 'A2', 'A1', 'A0']) <= params:
return self.sapm_spectral_loss
elif ((('Technology' in params or
'Material' in params) and
(pvsystem._infer_cell_type() is not None)) or
'first_solar_spectral_coefficients' in params):
return self.first_solar_spectral_loss
else:
raise ValueError('could not infer spectral model from '
'system.module_parameters')
'system.module_parameters. Check that the '
'parameters contain valid '
'first_solar_spectral_coefficients or a valid '
'Material or Technology value')

def first_solar_spectral_loss(self):
raise NotImplementedError

self.spectral_modifier = self.system.first_solar_spectral_loss(
self.weather['precipitable_water'],
self.airmass['airmass_absolute'])
return self

def sapm_spectral_loss(self):
self.spectral_modifier = self.system.sapm_spectral_loss(
self.airmass['airmass_absolute'])
Expand Down
81 changes: 81 additions & 0 deletions pvlib/pvsystem.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,87 @@ def sapm_effective_irradiance(self, poa_direct, poa_diffuse,
poa_direct, poa_diffuse, airmass_absolute, aoi,
self.module_parameters, reference_irradiance=reference_irradiance)

def first_solar_spectral_loss(self, pw, airmass_absolute):

"""
Use the :py:func:`first_solar_spectral_correction` function to
calculate the spectral loss modifier.
Copy link
Member

@wholmgren wholmgren May 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc string should have some kind of reference to self.module_parameters. Most of the other PVSystem method doc strings have some examples. This method is a little more complicated though, so might be worth being more explicit about what is looked for. For example...

Use the :py:func:first_solar_spectral_correction function to calculate the spectral loss modifier. The spectral loss modifier is determined by searching for one of the following keys in self.module_parameters (in order): 'first_solar_spectral_coefficients', 'Technology', 'Material'.

Copy link
Member Author

@cwhanse cwhanse May 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

    Use the :py:func:`first_solar_spectral_correction` function to
    calculate the spectral loss modifier. The model coefficients are
    specific to the module's cell type, and are determined by searching
    for one of the following keys in self.module_parameters (in order):
        'first_solar_spectral_coefficients (user-supplied coefficients)
        'Technology' - a string describing the cell type, can be read from
        the CEC module parameter database
        'Material' - a string describing the cell type, can be read from
        the Sandia module database.


Parameters
----------
pw : array-like
atmospheric precipitable water (cm).

airmass_absolute : array-like
absolute (pressure corrected) airmass.

Returns
-------
modifier: array-like
spectral mismatch factor (unitless) which can be multiplied
with broadband irradiance reaching a module's cells to estimate
effective irradiance, i.e., the irradiance that is converted to
electrical current.
"""

if 'first_solar_spectral_coefficients' in self.module_parameters.keys():
coefficients = \
self.module_parameters['first_solar_spectral_coefficients']
module_type = None
else:
module_type = self._infer_cell_type()
Copy link
Member

@wholmgren wholmgren May 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

module vs. cell is unfortunate here. No objection to what you've done, but we might need a separate discussion to try to clean up the terminology within the library.

Copy link
Member Author

@cwhanse cwhanse May 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I'm inclined to break the API and change the keyword argument from module_type to cell_type. module_type is only used by the first_solar function.

coefficients = None

return atmosphere.first_solar_spectral_correction(pw,
airmass_absolute,
module_type,
coefficients)

def _infer_cell_type(self):

"""
Examines module_parameters and maps the Technology key for the CEC
database and the Material key for the Sandia database to a common
list of strings for cell type.

Returns
-------
cell_type: str

"""

_cell_type_dict = {'Multi-c-Si': 'multisi',
'Mono-c-Si': 'monosi',
'Thin Film': 'cigs',
'a-Si/nc': 'asi',
'CIS': 'cigs',
'CIGS': 'cigs',
'1-a-Si': 'asi',
'CdTe': 'cdte',
'a-Si': 'asi',
'2-a-Si': None,
'3-a-Si': None,
'HIT-Si': 'monosi',
'mc-Si': 'multisi',
'c-Si': 'multisi',
'Si-Film': 'asi',
'CdTe': 'cdte',
'EFG mc-Si': 'multisi',
'GaAs': None,
'a-Si / mono-Si': 'monosi'}

if 'Technology' in self.module_parameters.keys():
# CEC module parameter set
cell_type = _cell_type_dict[self.module_parameters['Technology']]
elif 'Material' in self.module_parameters.keys():
# Sandia module parameter set
cell_type = _cell_type_dict[self.module_parameters['Material']]
else:
cell_type = None

return cell_type


def singlediode(self, photocurrent, saturation_current,
resistance_series, resistance_shunt, nNsVth,
ivcurve_pnts=None):
Expand Down
21 changes: 8 additions & 13 deletions pvlib/test/test_modelchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,21 +263,16 @@ def constant_spectral_loss(mc):
mc.spectral_modifier = 0.9

@requires_scipy
@pytest.mark.parametrize('spectral_model, expected', [
('sapm', [182.338436597, -2.00000000e-02]),
pytest.mark.xfail(raises=NotImplementedError)
(('first_solar', [179.371460714, -2.00000000e-02])),
('no_loss', [181.604438144, -2.00000000e-02]),
(constant_spectral_loss, [163.061569511, -2e-2])
])
def test_spectral_models(system, location, spectral_model, expected):
@pytest.mark.parametrize('spectral_model', ['sapm', 'first_solar', 'no_loss'])
Copy link
Member

@wholmgren wholmgren May 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please keep constant_spectral_loss in the list of parameters. This tests that a user-supplied function can be passed in as an argument. To be clear, the list should be: ['sapm', 'first_solar', 'no_loss', constant_spectral_loss]

Copy link
Member

@adriesse adriesse May 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

constant_spectral_loss --> constant_loss ?

Copy link
Member Author

@cwhanse cwhanse May 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put the keyword back. I tend to agree with @adriesse about 'loss' but the terminology is common. Maybe the time to alter usage is when (if) we revise function naming patterns, because changing 'loss' will alter the API.


def test_spectral_models(system, location, spectral_model):
times = pd.date_range('20160101 1200-0700', periods=3, freq='6H')
weather = pd.DataFrame(data=[0.3, 0.5, 1.0], index=times, columns=['precipitable_water'])
mc = ModelChain(system, location, dc_model='sapm',
aoi_model='no_loss', spectral_model=spectral_model)
times = pd.date_range('20160101 1200-0700', periods=2, freq='6H')
ac = mc.run_model(times).ac

expected = pd.Series(np.array(expected), index=times)
assert_series_equal(ac, expected, check_less_precise=2)
spectral_modifier = mc.run_model(times=times, weather=weather).spectral_modifier
print(spectral_modifier)
Copy link
Member

@wholmgren wholmgren May 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I try to avoid putting print functions in the final tests. In case you are not already aware of it, the pytest --pdb option can be useful for avoiding print in development e.g. pytest pvlib/test/test_modelchain.py::test_spectral_models --pdb

Copy link
Member Author

@cwhanse cwhanse May 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed, I had that print to debug the test.

assert isinstance(spectral_modifier, (pd.Series, float, int))


def constant_losses(mc):
Expand Down
11 changes: 11 additions & 0 deletions pvlib/test/test_pvsystem.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,17 @@ def test_PVSystem_sapm_spectral_loss(sapm_module_params):
out = system.sapm_spectral_loss(airmass)


@pytest.mark.parametrize("expected", [1.03173953])

Copy link
Member

@wholmgren wholmgren May 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python convention is no blank lines between decorators and the function.

Copy link
Member Author

@cwhanse cwhanse May 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

def test_PVSystem_first_solar_spectral_loss(sapm_module_params, expected):
system = pvsystem.PVSystem(module_parameters=sapm_module_params)
pw = 3
airmass_absolute = 3
out = system.first_solar_spectral_loss(pw, airmass_absolute)

assert_allclose(out, expected, atol=1e-4)


@pytest.mark.parametrize('aoi,expected', [
(45, 0.9975036250000002),
(np.array([[-30, 30, 100, np.nan]]),
Expand Down