Skip to content

Issue 84: Created a tutorial with system losses and multiple strings. #88

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
wants to merge 1 commit into from

Conversation

jforbess
Copy link
Contributor

I've created a tutorial notebook (system_loss_modeling.ipynb) that pulls in system configuration information from a CSV file, and creates three different systems. It defines a system model method to create the system and apply the losses, and then graphs the output vs poa. All of the systems are using Sacramento TMY3 data.

Please let me know if this is not basic enough, or not documented enough, because I just simplified and documented code I had written, and it makes a lot of assumptions of understanding, I am sure.

Also, I consider this tutorial a starting point in setting up system config and loss components that may help us flesh out #17 better. I don't think that the meta dictionary I have defined is the best way, but I'd like to use it to improve pvsystem.systemdef().

@wholmgren
Copy link
Member

Thanks!

Here are links to the nbviewer version of the notebook and the csv file:

http://nbviewer.ipython.org/github/jforbess/pvlib-python/blob/Issue84/docs/tutorials/system_loss_modeling.ipynb

https://github.com/jforbess/pvlib-python/blob/Issue84/pvlib/data/site_config.csv

A few quick thoughts:

Have you considered or used json or yaml config files? yaml files can be easy to read, but I don't have experience parsing them. I ask because csv files are hard to write by hand, though they're easy to write/read/edit with something like excel.

Looks like we're going to need to do some more work on name standardization.

I think you can make your 2nd cell (currently marked [30]) a little easier to read and maintain if you do something like:

for site in site_config.iterrows():
    # site should be a series indexed by strings
    # now access with site['Latitude'] etc.

I'll make some more detailed comments later this week when I have more time to think.

@jforbess
Copy link
Contributor Author

Thanks for the feedback.

I would like to support CSV site_config files because they are pretty much
the normal way of doing business with non-coders, who are usually the
people responsible for compiling this information from operating plants.
One of the reasons I find pvlib valuable is its ability to support a large
portfolio, so I have been thinking about this from evaluating many plants
at once. It may be more appropriate to just load the site_config data as a
DataFrame and access the relevant field with meta[site]['Column Name']
rather than trying to translate into a weird meld of a Location, systemdef,
and other info structure. Right now I'm not really sure what the advantage
to having the current Location and systemdef setup is (as noted in #17).

Your documentation example is helpful, and I'll make that adjustment along
with others you suggest this week.

On Tue, Sep 29, 2015 at 10:48 AM, Will Holmgren [email protected]
wrote:

Thanks!

Here are links to the nbviewer version of the notebook and the csv file:

http://nbviewer.ipython.org/github/jforbess/pvlib-python/blob/Issue84/docs/tutorials/system_loss_modeling.ipynb

https://github.com/jforbess/pvlib-python/blob/Issue84/pvlib/data/site_config.csv

A few quick thoughts:

Have you considered or used json or yaml config files? yaml files can be
easy to read, but I don't have experience parsing them. I ask because csv
files are hard to write by hand, though they're easy to write/read/edit
with something like excel.

Looks like we're going to need to do some more work on name
standardization.

I think you can make your 2nd cell (currently marked [30]) a little easier
to read and maintain if you do something like:

for site in site_config.iterrows():
# site should be a series indexed by strings
# now access with site['Latitude'] etc.

I'll make some more detailed comments later this week when I have more
time to think.


Reply to this email directly or view it on GitHub
#88 (comment).

@wholmgren
Copy link
Member

Sorry @jforbess for the very long delay on this.

I go back and forth on the right levels of abstraction and encapsulation for pvlib. It's super helpful for me to see how you're using it, so thanks again for sharing. I put most of my comments regarding this in a comment in #17.

Another minor suggestion for your code: you might want to checkout pd.to_numeric to reduce the need to call float and int on your strings.

After the changes discussed here and above for readability, I think that your tutorial would be a nice addition.

@wholmgren
Copy link
Member

I still like the idea of adding a notebook like this, but I'm guessing that it's better to start from scratch after 1+ years of pvlib improvements and api changes.

@wholmgren wholmgren closed this Feb 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants