Gb/bc #148
Gb/bc #148
Conversation
rex/sam_resource.py
Outdated
|
|
||
| Parameters | ||
| ---------- | ||
| bc_df : None | pd.DataFrame |
There was a problem hiding this comment.
Can bc_df actually be None? Seems like that would error out (and also not be very useful). My guess is this is leftover text from a pervious version?
Also what do you mean by "extracted DataFrame"?
There was a problem hiding this comment.
I'm really good at copy pasta. Extracted is left over bc at some point it's either a string filepath or "extracted" e.g. read from the filepath
rex/sam_resource.py
Outdated
| None if not provided or extracted DataFrame with wind or solar | ||
| resource bias correction table. This has columns: gid, adder, | ||
| scalar. If either adder or scalar is not present, scalar defaults | ||
| to 1 and adder to 0. Only windspeed, GHI, and DNI are corrected. |
There was a problem hiding this comment.
So I was surprised that the code only corrects windspeed or GHI + DNI. In retrospect it makes sense, but I think the docstring misled me into thinking the code should do something it does not intend to do.
There was a problem hiding this comment.
Yep good point, will edit.
rex/sam_resource.py
Outdated
| scalar = np.expand_dims(bc_df.loc[site_arr[isin], 'scalar'].values, 0) | ||
| adder = np.expand_dims(bc_df.loc[site_arr[isin], 'adder'].values, 0) | ||
|
|
||
| if 'windspeed' in self._res_arrays: |
There was a problem hiding this comment.
This almost does not work for NSRD. Are you intentionally relying on the implementation detail of solar data to include an underscore in the variable name (i.e. wind_speed instead of windspeed)? It's possible this is a standard I am not aware of, but it could lead to problems down the line otherwise.
One solution that may not be too much extra work to implement is to ask explicitly for a wind or a solar bc_df (or both!):
def bias_correct(self, wind=None, solar=None):
...
Then in reV, users could just specify bias_correct={"wind": "/path/to/wind/bc_df.csv", "solar": "/path/to/solar/bc_df.csv"}, and the _parse_bc function could take care of loading in the DataFrames to pass to this function.
Anyways, I won't insist on anything but just some food for thought
There was a problem hiding this comment.
Oh wow haha you're totally right, this could have easily went to wind for solar data and skipped the irrad vars. Easier fix is just to check for 'ghi' first which is the more precise condition.
No description provided.