ustarfilter bug fixes?#4
Conversation
* added a test for ustarfilter.py * instead of an annual minimum of 360 days of data per year, now using a seaonal threshold: seasons/periods with fewer than minperioddata nighttime datapoints will be skipped.
* undef values are now converted to NaNs right away, so that we only need to deal with np.nan * isday can be bool or float or int: documentation was unclear about the exact datatype needed. * values that can be NaN are now cast to floats where necessary
If there is not enough data in a given season to estimate ustar, we default to either * the maximum ustar threshold of the other seaons * the default ustar value, if provided * throwing an error, if no valid seasons exist also continued to improve nan handling
mcuntz
left a comment
mcuntz
left a comment
There was a problem hiding this comment.
Dear Alex,
thanks for the bugfixes and enhancements. I added comments and suggestions.
In general for hesseflux: I think that gapfilling, spike detection and ustar filtering are generally o.k. (small bugs included like the ones you found).
Partitioning (in nee2gpp.py), however, is less well tested and there are definitely errors in there. Use it with care, and I will be happy about any improvements.
Kind regards,
Matthias
| astr = 'Input must be either numpy.ndarray or pandas.DataFrame.' | ||
| assert isinstance(dfin, pd.core.frame.DataFrame), astr | ||
| df = dfin.copy(deep=True) | ||
| df = dfin.copy(deep=True).astype(np.float32) |
There was a problem hiding this comment.
Do not change all the DataFrame to float32. It could contain a lot of other unrelated columns (e.g. datetime) so that this fails. You do not need it anyway.
| assert isinstance(dfin, pd.core.frame.DataFrame), astr | ||
| df = dfin.copy(deep=True) | ||
| df = dfin.copy(deep=True).astype(np.float32) | ||
| df = df.replace(undef, np.nan) |
There was a problem hiding this comment.
np.nan is type float, which corresponds to np.float64. So np.float32 is not good in any case.
| assert sw_id, astr | ||
| # Papale et al. (Biogeosciences, 2006): 20; REddyProc: 10 | ||
| isday = df[sw_id] > swthr | ||
| isday = (df[sw_id] > swthr).astype(np.float32) |
There was a problem hiding this comment.
See above. Should be simply float instead of np.float32.
| isday = isday.to_numpy() | ||
| isday[isday == undef] = np.nan | ||
| ff[np.isnan(isday)] = 1 | ||
| isday = np.atleast_1d(isday).astype(np.float32) |
There was a problem hiding this comment.
Same: float instead of np.float32.
|
|
||
| isday[(isday == undef) | (np.isnan(isday))] = np.nan | ||
| isday = isday.flatten() | ||
| ff.loc[np.isnan(isday), :] = 1 |
There was a problem hiding this comment.
I would suggest to add the line:
isday = isday.astype(bool)
here so that we do not have to do isday_b.astype(bool) all the time.
My numpy version simply set NaN to True in this conversion.
| May 2026, Alex Fox | ||
| """ | ||
| import warnings | ||
|
|
| isday : array_like of bool, optional | ||
| True when it is day, False when night. Must have the same length as | ||
| isday : array_like, optional | ||
| 1/True when it is day, 0/False when night. May contain nan/undef values. Must have the same length as |
There was a problem hiding this comment.
Please try to stay below 80 characters per line.
| mindaysperyear : int, optional | ||
| Minimum number of days of data per year to avoid warning (default: 360). Datasets with fewer days per year may give unreliable results. | ||
| ustardefault : float, optional | ||
| Default ustar threshold to use for seasons with insufficient data (default: None). If None, the maximum ustar threshold of the other seasons is used. |
There was a problem hiding this comment.
Please try to stay below 80 characters per line also in the docstring.
There was a problem hiding this comment.
Great for the test. Please rename it to test_ustarfilter.py.
|
Hi Matthias, thank you for your response and your edits -- I'll take a look and implement them sometime soon when I have a break in my other work. Happy to contribute to other parts of the project like further bughunting and the flux partitioning, too. Best, |
2 participants
Hi, I recently downloaded this package to integrate into my EC QA/QC python workflow so I don't have to use ReddyProc. Super handy, thank you for putting in the work to create this! However, I noticed several bugs that kept popping up when I tried to use it. I made some fixes on my local machine (python 3.11.14, numpy 2.3.5, pandas 2.3.3) that you might find helpful.
Bugs that I kept encountering:
~isday_b) would raise a TypeErrorisday, which is treated as an integer array with missing values.len(df)instead ofdf.shapewhen computing the number of datapoints in a dataframeI fixed these issues by
floatso that it can handle missing values, and casting toboolwhen doing logical operationsundefwithnp.nanright awaydf.shapewhere appropriateI also made a couple of quality-of-life changes for my own use, which are also included (but can be removed if you would like!):
I was running into an issue where the program would throw an AssertionError whenever I had fewer than 360 days per year in my dataset, on average. I understand that this is to ensure that the methodology works reliably, but I find that it's quite rare to have such a complete flux dataset. I think I could skirt around this by padding the data to extend from January 1 to December 31, but this was pretty cumbersome. Instead, I added a few additional parameters:
mindaysperyearargument is used instead of the hardcoded 360 day cutoffminseasondataargument makes it so that seasons/periods with fewer thanminseasondatanighttime datapoints will not have a ustar threshold calculated for themI also added in a
randomstateargument that sets the prng seed in numpy, for reproducability.Thanks for making this very helpful package, and please let me know if this contribution would be useful, and if I should make any changes!
-Alex