pvlib / pvlib-python

A set of documented functions for simulating the performance of photovoltaic energy systems.
https://pvlib-python.readthedocs.io
BSD 3-Clause "New" or "Revised" License
1.16k stars 980 forks source link

meta, location, and systemdef philosophy #17

Closed jforbess closed 8 years ago

jforbess commented 9 years ago

I don't fully understand the differences of meta, location, and systemdef, in terms of how they should be used.

systemdef expects TMY meta to be used in its first parameter. But what if I want to use the lat/long for my plant, not the TMY station? What if I am looking at operating data (which I am), and have no need to pull in a TMY file?

Why doesn't systemdef use a location instead of a TMY meta as its first parameter? (Why isn't a TMY meta a Location object?)

wholmgren commented 9 years ago

These are all good points/questions. This is definitely a place where we need to improve the package.

I propose that we:

For 0.1 (#15), I think that we should just change the tmy_meta argument name to be meta and clarify the docs (#16) to specify that it should just be a dict that has these particular keys. There's nothing special about a dict that comes from the TMY importers vs. a dict that is user generated.

jforbess commented 9 years ago

Great, that is what I was thinking, but I wasn't sure there was something I was missing. Both the 0.1 and 0.2 solutions make sense to me.

I am interested in helping on small fixes like this, but as I'm an advanced beginner in python, and a beginner in github, is there a good reference doc for the process preferred by this repo of making some of these changes and presenting them for review?

wholmgren commented 9 years ago

@jforbess would you like to make a pull request with the 0.1 fixes?

Should probably add a test_systemdef_dict function to test_pvsystem to prove that it works as expected.

jforbess commented 9 years ago

@wholmgren sure, I'll do that, but not until Sunday or Monday.

wholmgren commented 9 years ago

@bmu and @Calama-Consulting do you support the proposal above or do you have other ideas for how to resolve this? I think is a high priority for 0.2 since it could impact the API in fairly significant ways.

@jforbess, sorry, just now realizing that I did not respond to your question regarding contributing. Does the Development information wiki help?

bmu commented 9 years ago

As mentioned by @jforbess I think that one often uses a lat/lon different from the TMY data. So I am not really sure, if it is a good idea to return the Location from the TMY reader. This may be different for other data sources, e.g. measurement data or satellite data.

A System or perhaps better a PVSystem class would be really nice. However I wouldn't derive it from Location because it is something completely different. And it is a very complicated task to develop something that covers all aspects. Here are some points, that come to my mind:

We cannot cover all of this in pvlib (maybe this would be a good topic for a PhD and I think it would be very useful to have a standard for the whole PV community). And up to now we don't need all of this (e.g. we don't have models for calculation of shading losses, wiring losses, transformers ...). But maybe we should have it on mind when we implement something like this.

And I agree, this is something important for our API, so we should have something quite stable for the future.

bmu commented 9 years ago

My comment in #39 is possibly related (is it possible to link to a specific comment?)

wholmgren commented 9 years ago

Thanks, those comments are a nice place to start the discussion.

It was pretty natural for me to think about PVSystem inheriting from Location since I always think about a PV system as being located somewhere on the globe. One place where this model breaks down is in indoor testing environments, for cells, modules, and small arrays. In any case, my real motivation was to inherit some of the constructor code, so not a big deal.

Another thing to keep in mind is that there might be situations where some models can only be used with certain kinds of systems.

At the risk of getting off topic... I'm not sure why it matters that you may want to use the TMY data for a nearby but different location. In going from TMY to systemdef or similar you're stuck with updating a dict, a Location object, or manually specifying new lat/lons no matter what. I always like using built-ins, Location provides some consistency with the Matlab code and maybe is more concrete and extensible, and you can't beat manual specifications for simplicity. There's no clear winner in my mind.

I'm not really sure where to start here. Should we mock up some objects and example code as in @bmu's comment? (Click on the date of the comment to get a link to it.)

wholmgren commented 8 years ago

Thanks to @jforbess for creating a new tutorial in #88 and making me think about this again.

In the spirit of retaining the low-level, mostly non-object-oriented nature of pvlib, I think that we should could expand the definition of system_def to be "a general place to put crap about your PV system". If you use key words from the pvlib name list, then it will work when you **system_def into pvlib functions, but otherwise you can use the dict however you want. We could reduce the complexity of a lot of user code by allowing most pvlib methods to take **kwargs for the unused parts of the system_def dict. On a related note, dictionary unpacking got a lot more useful in Python 3.5 with the addition of more flexible unpacking tools (PEP 448), so you can do things like

pvlib.some_method(**{**system_def, **my_other_params})

I now think that methods such as get_solarposition should only take lat, lon, and tz arguments instead of a Location object. Furthermore, the Location objects would have a function that calls the get_solarposition method with the appropriate arguments based on the object attributes. So you could accomplish the same thing in two ways:

site = pvlib.location.Location(lat, lon, tz, elev, name)
solpos = site.get_solarposition(times)

or

site = {'lat':lat, 'lon':lon, 'tz':tz, 'elev':elev, 'name':name}
solpos = pvlib.solarposition.get_solarposition(**site)

To make the object oriented stuff work using the procedural code we can do things like

class Location(object):
    # stuff like init

    def get_solarposition(self, times):
        return pvlib.solarposition.get_solarposition(times, self.lat, self.lon)

    # maybe other wrapper stuff for other module functions

class PVSystem(Location):
    def __init__(module_type, other_system_crap, args_and_kwargs_for_Location):
        self.module_type = module_type 
        # and so on for the rest of other_system_crap
        # maybe do something to make non-Location systems (i.e. indoor testing) reasonable
        super().__init__()

    def calcparams_desoto(self, poa_globa, temp_cell):
         return pvsystem.calcparams_desoto(poa_global, temp_cell, module_parameters=self.module_type)

    def singlediode(self, params)
        return pvsystem.singlediode(attributes, params)

    # remember that get_solarposition was inherited from Location, too

I worry about the maintenance cost for this, but maybe it would be worth it. @bmu objected to this inheritance, so that needs to be readdressed too.

I also found this post by Matthew Rocklin to be helpful in thinking about dictionaries vs. classes.

wholmgren commented 8 years ago

I forgot that clearsky.ineichen also takes a Location object as an argument. The same changes would be applied here.

Current:

def ineichen(time, location, linke_turbidity=None, 
             solarposition_method='pyephem', zenith_data=None,
             airmass_model='young1994', airmass_data=None,
             interp_turbidity=True):

Proposed:

class Location(object):
    #other stuff here as described above
    def ineichen(time, **kwargs):
        return clearsky.ineichen(time, self.latitude, self.longitude, **kwargs)

def ineichen(time, latitude, longitude, linke_turbidity=None, 
             solarposition_method='pyephem', zenith_data=None,
             airmass_model='young1994', airmass_data=None,
             interp_turbidity=True):
jforbess commented 8 years ago

I meant to respond to your earlier thoughts about this, but time slipped away.

I definitely agree that Location doesn't seem to be an especially useful object, and that a dict makes more sense. I need to do a bit more research on the links you provided, because I think I am glossing over some of the value of the different ways to abstract this.

wholmgren commented 8 years ago

It's been long enough now that I've had to remind myself of why things are the way they are. I figured that I might as well put it the results here...

The Matlab toolbox uses location structs in a few places, e.g.:

https://github.com/Sandia-Labs/PVLIB_Matlab/blob/master/PVLIB/pvl_makelocationstruct.m

https://github.com/Sandia-Labs/PVLIB_Matlab/blob/master/PVLIB/pvl_clearsky_ineichen.m

https://github.com/Sandia-Labs/PVLIB_Matlab/blob/master/PVLIB/pvl_ephemeris.m

Rob's original Python port copied most of this behavior:

https://github.com/Sandia-Labs/PVLIB_Python/blob/master/pvlib/pvl_makelocationstruct.py

https://github.com/Sandia-Labs/PVLIB_Python/blob/master/pvlib/pvl_clearsky_ineichen.py

https://github.com/Sandia-Labs/PVLIB_Python/blob/master/pvlib/pvl_ephemeris.py

One place where the Matlab code and the original Python code diverged was in the TMY reader results, also known as "meta". The Matlab version put data and metadata into the same struct, where as the Python version had to break out the metadata into a dict so that the data could be in a DataFrame.

I wanted PVLIB_Python to use the patterns and terminology of a standard Python library, so I created a PR (Sandia-Labs/PVLIB_Python#17) that replaced the location struct with a Location class.

In retrospect, I believe that was a missed opportunity to drop the location object/struct arguments in favor of latitude, longitude, tz arguments.

I created a prototype PR #93 to help make the discussion about the future more concrete.

dacoex commented 8 years ago

Commenting on https://github.com/pvlib/pvlib-python/issues/17#issuecomment-152287032 after having gone through https://github.com/pvlib/pvlib-python/pull/93#issuecomment-155255580

It seems that Location and PVSystem as object will have quite some overhead:

Is this correct?

What would be the benefit?

Would it allow me to have systems that inherit some parameters (tilt angle) while changing others (module type)?

wholmgren commented 8 years ago
  • write the function
  • insert the function call to the 2 objects above (and not forget it)

Is this correct?

No. There are two classes, but one inherits from the other. You write the module level function and then you add a function to the appropriate class. Whether or not this is a significant overhead is relative to the benefits/drawbacks of maintaining both procedural and object-oriented code.

dacoex commented 8 years ago

You write the module level function and then you add a function to the appropriate class.

Yes, that's what I meant.

Whether or not this is a significant overhead is relative to the benefits/drawbacks of maintaining both >procedural and object-oriented code.

So with the object code one would be able to calculate some default settings for the PVSystem class such as the optimum tilt angle as default etc?

wholmgren commented 8 years ago

So with the object code one would be able to calculate some default settings for the PVSystem class such as the optimum tilt angle as default etc?

If you add that functionality to the object code, then it would be able to do that. There is nothing stopping you from writing an optimizer using the procedural code either. The object code would just be a different way of interacting with pvlib.

cwhanse commented 8 years ago

Yes to calculating defaults in general but no for your example – optimum is usually defined by maximizing annual energy, which will depend on weather, shading, etc., that aren't properties of the PV system.

I never liked the Location structure in Matlab PV_LIB, just for the record.

dacoex commented 8 years ago

I never liked the Location structure in Matlab PV_LIB, just for the record.

@cwhanse so would you propose sth. different than @wholmgren ?

cwhanse commented 8 years ago

I don't know what sth is.

dacoex commented 8 years ago

something

wholmgren commented 8 years ago

I never liked the Location structure in Matlab PV_LIB, just for the record.

Thanks @cwhanse. That is very helpful to hear. After working on #93 last night I am leaning towards suggesting that it be removed entirely from the Python version.

cwhanse commented 8 years ago

Ah. No I would not.

In the PV_LIB Matlab context the Location structure served only to pass three arguments (lat, long and altitude) with a single variable. The shortcut didn't seem worth the bother of having a structure. When there's many variables that have to stay together (like coefficients for a PV module model, or a measured IV curve) then a structure made sense.

wholmgren commented 8 years ago

I think that we may actually be in agreement so let me try to clarify.

The half-baked code that I'm showing in #93 introduces a new PVSystem class that currently inherits all of the Location class attributes and methods. The idea of the PVSystem class is to provide a formal container like the one you say makes sense (I think). I'm now thinking that this Location --> PVSystem class inheritance in the proposed code is unnecessary/confusing and we should just put all of the attributes and methods into one PVSystem class.

jforbess commented 8 years ago

I have always been confused as to the value of Location as a special object beyond the rest of the PVsystem, and would rather consider lat, long, and altitude as simply required attributes of a PVsystem. I know we talked about its value for a few functions, but as I will never model a clear sky without some PVsystem to go along with it, it doesn't seem useful to me.

So I am currently on board with Will, I think.

cwhanse commented 8 years ago

If I understand this correctly, you are now thinking that data that are intrinsic to a location (latitude, longitude, weather) will be subordinate to a PVsystem?

I think the two (PVsystem and Location) would be better separated. What if one was trying to optimize a design for a particular site, and wanted to “loop” over many PV systems?

But what I know about object-oriented code can be summarized in less text than this email.

cwhanse commented 8 years ago

I frequently run parts of the PV modeling sequence, including clear sky models, for purposes other than calculating power. For example, to investigate the quality of satellite irradiance data. I understand jforbess’ point, just a thought that there are broader uses for this code.

dacoex commented 8 years ago

What if one was trying to optimize a design for a particular site, and wanted to “loop” over many PV >systems?

Agree, also loop the same system through different sites.

wholmgren commented 8 years ago

These are good points. I don't know if there is a right answer to these problems.

Some more half-baked ideas to consider:

The diversity of opinions and use cases reinforces the need to keep the procedural code clean and flexible. Whatever object oriented API we ultimately go with will just be one possible solution.

wholmgren commented 8 years ago

Here's one way to loop over locations using the third idea in my previous comment.


base_system = PVSystem(module, inverter, **other_params)

coordinates = [(30, -110), (35, -100), (40, -120)]

times = pd.DatetimeIndex(start='2015-01-01', end='2016-01-01', freq='1h')

energies = []
for latitude, longitude in coordinates:
    localized_system = base_system.localize(latitude, longitude)
    cs = localized_system.get_clearsky(times)
    solpos = localized_system.get_solarposition(times)
    total_irrad = localized_system.get_irradiance(times, **solpos, **cs)
    power = localized_system.get_power(stuff)
    annual_energy = power.sum()
    energies.append(annual_energy)
bmu commented 8 years ago

From my point of view it is still the best idea to have a separated Location and PVSystem class without inheritance as I think they have nothing in common.

The Location class should handle and simplify all details on he location but also all meteorological data, (horizontal) irradiance, clear-sky, temperatures... Maybe the location class could contain a data attribute, which is a DataFrame. So all data can be calculated and stored within this DataFrame.

The PVSystem class should handle and simplify all details on the PV system. This is starting from horizontal irradiance and reaches to final energy yield. Maybe also this object should store all values in a data attribute

So one could write:

coordinates = [(30, -110), (35, -100), (40, -120)]
pv_system = PVSystem(module, inverter, **other_params)
times = pd.DatetimeIndex(start='2015-01-01', end='2016-01-01', freq='1h')
results = {}

for latitude, longitude in coordinates:
    location = Location(latitude, longitude) 
    location.get_data(source='TMY') 
    # or alternatively: source='some_file.csv', type='Meteonorm7' or 'SolarGIS' ...
    # alternatively:
    # location.get_clearsky(times)
    # location.get_solarposition(times)
    # location.get_irradiance(times, **solpos, **cs)
    pv_system.set_location(location)
    pv_system.calculate_power()
    # store all data from all modelling steps
    results[(latitude, longitude)] = pv_system.data
    # or store only summaries
    energy = pv_system.data['AC_power'].sum()
    gpoa = pv_system.data['GPOA'].sum()
    pr = energy / gpoa * 100.0
    results[(latitude, longitude)] = {'PR': pr, 'GPOA': gpoa, 'Energy': energy}

Same could be done when you want to cycle with different systems for a single location.

Also, maybe it would be useful to have another class ModelChain which defines all models to use for the simulation and contains some logic to decide which parameters to use at which conditions or locations (e.g. soiling dependent on climatic zones and / or tilt angle). This would give full flexibility and one could also change or cycle through different modeling steps, e.g. use different conversion models for tilted irradiance or so on. So there would be a few additional lines at the top of the example

    from pvlib.modelling_chains import DefaultModelingChain
    pv_system = PVSystem(module, inverter, **other_params)
    model_chain = DefaultModelingChain(conversion_model='Klucher')
    pv_system.model_chain = model_chain
cwhanse commented 8 years ago

From my point of view it is still the best idea to have a separated Location and PVSystem class without inheritance as I think they have nothing in common.

I agree

The Location class should handle and simplify all details on he location but also all meteorological data, (horizontal) irradiance, clear-sky, temperatures... Maybe the location class could contain a data attribute, which is a DataFrame . So all data can be calculated and stored within this DataFrame.

This would have made the Matlab Location structure useful.

Also, maybe it would be useful to have another class ModelChain which defines all models to use for the simulation and contains some logic to decide which parameters to use at which conditions or locations (e.g. soiling dependent on climatic zones and / or tilt angle).

I absolutely support documenting choice of models and parameters, but I'd tie that information to the calculation of energy yield. I'm not sure that creating a new class to contain inputs helps. If a new class is the way to go, I'd broaden that class so that an instance contains both the inputs and outputs from the energy simulation.

bmu commented 8 years ago

The idea of the ModelChain class is more about flexibility in modeling. could also be a function, but a class allows to change single modeling steps without the need to write a whole new function. And you have a dedicated class for all three influencing factors of the calculated energy yield: the PV system, the location and the model/models/parameters you used.

I think documenting the results is a different topic. At my institute we use to store the results and all parameters used to get these results in a single plain text file. This should be easy to implement for pvlib, too. Could be a method of PVSystem, something like save_results('my_results.txt' or 'my_results.hdf5' ..., kind='summary' or 'all_time_steps')with different possible output formats and levels of detail and a representation of all models, parameters, system details ... used.

dacoex commented 8 years ago

I would second the proposal put forward in https://github.com/pvlib/pvlib-python/issues/17#issuecomment-155868023

especially the ModelChain idea.

Let's get this started.

wholmgren commented 8 years ago

@jforbess: have @bmu and @cwhanse changed your mind?

@bmu's example brings up another design issue: side effects. Most of @bmu's example code mutates the state of the pv_system object. In contrast, all of my example code returned a DataFrame or a new object. There are pros and cons to each approach

Against side effects:

  1. Personal experience. Over the last couple of years I developed 10k+ lines of code that used objects that had a lot of side effects and it turned into a total disaster.
  2. Most of the core PyData stack avoids side effects.
  3. The Zen of Python says that explicit is better than implicit.
  4. You have to carefully read the documentation to figure out where the new result goes since everything just returns None.

For side effects:

  1. You don't have to worry about assignment and keep track of intermediate results.
  2. The code looks a little cleaner.
wholmgren commented 8 years ago

Regarding ModelChain, I think I'd need to see this used in some practical applications before I could say if I think it's a good idea. I too am all for coming up with some standards for saving both metadata (possibly including a serialization of a ModelChain object``) and results.

Let's get this started.

@dacoex This is a little frustrating to hear. What do you think #93 is if not an attempt to get this discussion moving in a more concrete direction?

bt- commented 8 years ago

I've been hoping to have more time to explore the library before commenting, but here are some poorly organized thoughts.

Allow latitude and longitude to be optional arguments to a PVSystem object, but keep its Location inheritance

I like this approach. I'm not sure I agree totally with @bmu and @cwhanse that a Location and PVsystem class do not have anything in common. I'm trying to remember how all the algorithms fit together to get from GHI to final energy yield. Looking at the TMY to Power tutorial, isn't it necessary to have location data to calculate POA values? The aoi and haydavies functions use data from solpos, which was calculated using an object of the Location class. If we have to have location data for PVsystem calculations and we want to be able to create location objects independent of a definition of PV system, doesn't it make sense to inherit Location attributes from Location rather than repeating code for it? I'm still learning PVlib, so these are honest questions not rhetorical.

I will put in my vote for avoiding side effects. I have minimal experience with development, but it makes sense to me to stick with the conventions of the other libraries in the PyData stack.

And you have a dedicated class for all three influencing factors of the calculated energy yield: the PV system, the location and the model/models/parameters you used

I'm not sure I fully understand what would differentiate the ModelChain from PVsystem. I would like to see the PVsystem class develop in a way that it has all the attributes necessary to run a 'simulation'. Maybe even have the methods necessary to calculate final energy yield called in the initialization of a PVsystem object. Although, this may run counter to my vote against side effects.

I think it would make sense to save out results data to a csv using pandas. This is the format used by other PV modeling tools. I agree with the idea of storing inputs and outputs together as a matter of practice, but I think in my own use I would prefer to save them in separate files located in the same directory.

dacoex commented 8 years ago

@dacoex This is a little frustrating to hear.

Sorry, that was not the intention here.

What do you think #93 is if not an attempt to get this discussion moving in a more concrete direction?

Yes, sure. But I am a little lost on what needs to be done to move that PR. And I feel until this discussion has come to a conclusion the way to continue this work would be uncertain. But we are very close to finalise the design choices and then can start implementing.

But what I know about object-oriented code can be summarized in less text than this email.

Same would apply for me case. At least when it comes to structuring the re-fractoring of a whole library. I will keep more silent and wait the decisison of those currently actively using the library and having more programming experience.

I will put in my vote for avoiding side effects. I have minimal experience with development, but it makes sense to me to stick with the conventions of the other libraries in the PyData stack.

This is good and that's why this discussion is useful. That done, all will be clear.

I'm not sure I fully understand what would differentiate the ModelChain from PVsystem. I would like to see the PVsystem class develop in a way that it has all the attributes necessary to run a >'simulation'.

There are other calculations not directly related to PV system: data quality checks, extended statistics and plotting, etc.

alorenzo175 commented 8 years ago

Here are some thoughts on how to move forward.

First, I am strongly opposed to any methods that cause side effects. Debugging or changing code that implicitly modifies objects is frustrating and time consuming.

I also don't see a need for PVSystem to inherit from Location. In my mind, they would each simply be initialized with attributes of the system or location (lat, long, inverter type, cell type, etc) and then define convenience methods that use these attributes to call the procedural code and return the result. This is basically the same as what @wholmgren proposes in https://github.com/pvlib/pvlib-python/issues/17#issuecomment-150969390 but with no Location inheritance.

Then, a ModelChain class could takes as inputs a PVsystem, Location, and some other arguments that specify which models to use. Then a run_model method can be called to run through the chain calling modeling functions with PVsystem and Location attributes to produce some final output. We could also define a method that calls run_model but also includes the input attributes and modeling arguments in one final output. For example,


class ModelChain(object):
    def __init__(self, system, location, solar_position_method, clearsky_method, times,...):
        self.system = system
        ...

    def run_model(self):
        solar_position = self.location.get_solarposition(self.times)
        clearsky = self.system.get_clearsky(solar_position, self.clearsky_method)
        ...
        final_output = self.system.calculate_final_yield(args)
        return final_output

    def model_system(self):
        final_output = self.run_model()
        input = self.prettify_input()
        modeling_steps = self.get_modeling_steps()
        # print/write out the input, modeling steps, and output to a file or in some format

It is pretty simple to just define a system, a location, and the models you want to use and do

mc = ModelChain(system, location, otherstuff)
output = mc.run_model()

Maybe we could add a convenience function to save everything or the user can do some pandas.to_csv() themselves.

bmu commented 8 years ago

This sounds good! Better this way, that the ModelChain takes system and location as arguments.

bt- commented 8 years ago

Tony's proposals for moving forward make sense to me. The structure and classes seem intuitive.

wholmgren commented 8 years ago

Thanks everyone for your thoughts on this issue.

I'm now seeing almost unanimous support for independent PVSystem and Location classes, so let's go in that direction first. I updated #93 based on this conclusion. I think that the Location class in #93 is ok (though not yet complete). The implementation details of the PVSystem class in #93 need more thought. I'll welcome pull requests to my branch or competing pull requests to pvlib/master.

@jforbess: my understanding is that you would have favored a single class that combines it all. It lieu of that we can also experiment with a LocalizedPVSystem that inherits from both. I think that it would be fairly easy to implement, but I don't have much experience with multiple inheritance. The internet is full of "never use multiple inheritance" advice, but I think that it could work out ok in this case. The following code seems to prove that it's possible to have reuse the same base PVSystem and Location classes in a composite LocalizedPVSystem class:

class PVSystem(object):
    def __init__(self, module, inverter, **kwargs):
        print('initializing a PVSystem')
        self.module = module
        self.inverter = inverter 
        super(PVSystem, self).__init__(**kwargs)

class Location(object):
    def __init__(self, latitude, longitude, **kwargs):
        print('initializing a Location')
        self.latitude = latitude 
        self.longitude = longitude 
        super(Location, self).__init__(**kwargs)

# inheritance order should not matter if defined as above
class LocalizedPVSystem(PVSystem, Location):
    def __init__(self, **kwargs):
        print('initializing a LocalizedPVSystem')
        super(LocalizedPVSystem, self).__init__(**kwargs)
        print('LocalizedPVSystem initialized')

In [17]: localized_pv_system = LocalizedPVSystem(latitude=30, longitude=-110, module='blah', inverter='blarg')
initializing a LocalizedPVSystem
initializing a PVSystem
initializing a Location
LocalizedPVSystem initialized

In [18]: localized_pv_system.latitude
Out[18]: 30

In [19]: localized_pv_system.module
Out[19]: 'blah'

In [21]: pv_system = PVSystem(module='blah', inverter='blarg')
initializing a PVSystem

In [22]: pv_system.module
Out[22]: 'blah'

In [23]: location = Location(latitude=30, longitude=-110)
initializing a Location

In [24]: location.latitude
Out[24]: 30

I do think that some people would find this to be a more useful paradigm than a combination of Location, PVSystem, and ModelChain. So, I am in favor of supporting both approaches provided that it's not too much extra work.

bmu commented 8 years ago

Just two thoughts, but mayby these are to advanced things at least for the moment:

First, for e.g. East West orientated modules and strings that are connected to a single Inverter / MPP tracker one would need Modeling chains that start with horizontal irradiance and model until DC. Than the results of these model chains must be combined and feed into a modeling chain that starts from the mpp tracker or inverter and model everything until final energy yield. So one would need ModelingChains that model not every step until final yield and the ModelingChain should be able to take results from other chains as arguments and combine them.

Second I was trying to integrate uncertainty calculation into our simulation and I endet up with classes that encapsulate every modeling step. These classes take a description oft their uncertainties and they are stacked into a modeling chain. So the chain has access to all uncertainties of their steps and can manage the propagation of uncertainties until final energy yield.

bt- commented 8 years ago

I would love to see both of these sets of functionality developed fully.

I see these potential features as areas where PVsyst and SAM fall somewhat short. I think SAM does have some decent capabilities for uncertainty analysis, but not the ability to propagate uncertainty through the model from inputs to final energy yield.

wholmgren commented 8 years ago

I am going to close this issue now #93 has been merged. I'm sure that we will need some new issues to discuss the modeling philosophy, the API and the implementation of the new features.