-
Notifications
You must be signed in to change notification settings - Fork 16
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
Post-process output from sig_RecessionAnalysis #5
Comments
Temporary solution has been worked out in this commit on the fork It saves the output variables as literal name of each parameters ( Also I have sorted out the conversion equation with a help from Hilary in my handwirtten note, but haven't coded them as comment yet |
|
Addressed in #14 |
This is great, thank you!! I have two questions. First, would it make sense to write a short signature function that calculates T0? Or provide it as an output with the sig_RecessionAnalysis function? That might perhaps be more consistent than the post-processing. And second, if we extract the median of the recession parameters a and b separately, this means they come from different recession segments. To get the parameters from the same recession, I used this code snippet: |
Thank you Sebastian for checking it through!
I agree either of the options would be more helpful and consistent! I wasn't entirely sure how the recession signature works with different options so I just went ahead with post processing for temporary solution.
Good idea to use it from the same recession, based on median b! It's more like you gonna pick a representative recession out of all recessions, rather than median of the parameters a and b, though. Should we phrase it like a and b from median curve or recession? I'll be out of office next 2 weeks (I'm actually writing this from an airport), so if you wanna work on the above changes please go ahead! If not I'm gonna pick up later |
TOSSH/TOSSH_code/calculation_functions/calc_McMillan_Groundwater.m
Lines 162 to 166 in a7012da
Issue description:
The current code
./TOSSH_code/calculation_functions/calc_McMillan_Groundwater.m
post-processes recession parameters (a
andb
) from individual fit fromsig_RecessionAnalysis
to derive the site mediana
andb
, as well as the timescale of recession decayT0
, that can be calculated froma
andb
. However, this post-processing is not included incalc_ALL.m
.This causes some confusions, as:
RecessionParameters
is a 1-by-2 matrix if you usecalc_ALL.m
and don't postprocess, and 1-by-3 matrix if you usecalc_McMillan_Groundwater.m
and post-process the parametersRecessionParameters
is a 1-by-2 or 1-by-3 matrix, whose element get converted as "RecessionParmaeters_1", "RecessionParmaeters_2", and "RecessionParmaeters_3" in output CSV file by default in Matlab. Therefore, in the output CSV file, it is not labeled which item represent what parameter, eithera
,b
, orT0
.T0
is a scaled timescale of recession by flow magnitude, but the process is not described in comments.I'm not looking for an immediate fix right now—left the comment for future improvement.
Some improvements to be done:
calc_ALL.m
,calc_McMillan_Groundwater.m
), instead of storing the output asRecessionParameters
(n-by-2 or n-by-3 matrix ), prepare output variables as literal name of each parameters (n-by-1 matrix for each:RecessionParameters_a
,RecessionParameters_b
, andRecessionParameters_T0
) and take the an element ofRecessionParameters
output like:./TOSSH_code/calculation_functions/calc_All.m
as well, or (2) included as an option insig_RecessionAnalysis
.a
parameter; it scalesa
according to flow magnitude (in other words,a
gets normalized by median flow) as described in McMillan et al., (2014).b
is (supposed to be) insensitive to flow magnitude, so no scaling needed.The text was updated successfully, but these errors were encountered: