Skip to content

Expose first_solar_spectral_correction in ModelChain and PVSystem #359

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.

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

Closed
wholmgren opened this issue Aug 7, 2017 · 4 comments
Closed

Comments

@wholmgren
Copy link
Member

wholmgren commented Aug 7, 2017

I suggest the following steps to expose the first_solar_spectral_correction function to ModelChain:

  1. Add a PVSystem.first_solar_spectral_correction(pw, airmass_absolute) method that wraps the atmosphere.first_solar_spectral_correction(pw, airmass_absolute, module_type=None, coefficients=None) function. This is straightforward except for deciding how to handle the keyword arguments. The Sandia module database has a Material key and the CEC database has a Technology key. These keys could be mapped to the allowed module_type arguments. I'm not sure what to do about the coefficients option. Perhaps direct users to put their custom coefficients in module_parameters['fs_spectral_coefficients']?

  2. Implement the stub ModelChain.first_solar_spectral_loss method. Maybe something like:

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

I'm guessing that most users don't have PW in their weather dataframes, so this line is going to be a frequent source of errors. We might consider adding a try/except KeyError with a more helpful error message.

  1. Improve ModelChain.infer_spectral_model. This probably amounts to setting mc._spectral_model = mc.first_solar_spectral_loss if a CEC module or a module_parameters['fs_spectral_coefficients'] key is detected on the PVSystem object.

  2. Optional. Add a dewpoint temperature to precipitable water conversion to the ModelChain.prepare_inputs method.

  3. Add tests.

  4. Update documentation.

It's unfortunate that sapm_spectral_loss and first_solar_spectral_correction live in different modules. Fixing that could be step 0, or we could leave it alone.

I'm in no rush to implement this, but I'd help anyone with a pull request.

see also #358

@wholmgren wholmgren added this to the 0.6.0 milestone Aug 7, 2017
@KonstantinTr
Copy link
Contributor

I'd like to takle this task. But I might need some help with taking decisions how it would be more convenient for users.

@cwhanse
Copy link
Member

cwhanse commented May 12, 2018

@KonstantinTr are you still interested to make this pull request?

@KonstantinTr
Copy link
Contributor

I'm truly sorry, I can't manage to find time to deliver this :(
I've recently changed industry and coutry of residence..

@cwhanse
Copy link
Member

cwhanse commented May 21, 2018

No worries, I didn't want this issue to stay unresolved, Many, many thanks for your contributions, and best wishes for the future.

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

No branches or pull requests

3 participants