NREL / bifacialvf

Bifacial PV View Factor model for system performance calculation
https://bifacialvf.readthedocs.io
Other
29 stars 18 forks source link

Pandas as input to simulate #25

Closed tcapelle closed 4 years ago

tcapelle commented 4 years ago

I am proposing some changes to the simulate function, mainly to be able to ingest pandas dataframes as inputs with precomputed sunpositions. This is needed to be able to compare against other models. I separated simulate in two functions:

I have also added a new notebook showing the new API in 'docs/example_simulate_inner.ipynb' Also, added a INES tmy file in /data/tmy_ines.csv

Some minor changes:

tcapelle commented 4 years ago

I don't get what the option: - REQ_ENV='--upgrade --upgrade-strategy=eager .' does and why it does not install tqdm. Ok added tqdm to setup.py and it passes now.

shirubana commented 4 years ago

I reviewed most of the changes, the only one I don't aggree on is the adding of the height (C) on the ideal rowspacing calculation (rowSpacing). This function is kinda useless anyways, but it tells you what separation "D" you need from the end of one module to the beginning of the next one for avoiding self-shading in fixed modules, which is independent of the clearance height of all the rows.

For example, if modules are tilted at 0, then D = 0. But with the addition of C, you don't get D = 0 ... etc.

I started changing how we deal with sunposition, I might merge your branch to mine. I however I have to discuss with @cdeline the bulk _simulate a sub-function of simulate .... I personally would prefer to give the user the ability of calling simulate with the dataframe already read on their own terms, or otherwise have that readTMY function kick in if the dataframe is not a dataframe and is an address on the computer. This is because I have my own versions of 'simulate' working with SRRL and pvdaq data and SNL data... so it's different every time :Q

tcapelle commented 4 years ago

Do you think this will be merged soon? The most important feature is a simulate function that takes a dataframe as input. @cdeline can you take a look at this, I really need this change to integrate bifacialvf on our toolbox.

cdeline commented 4 years ago

It sounds like these features are covered in PR #30 and will be included in the next release. I'm closing this for now, can re-open if the new release doesn't meet your needs.