Create _streamflow_flow_indices.py #1832
Conversation
streamflow_flow_indice is a submodule in the xclim indices module to calculate streamflow signatures, representing individual watershed characteristics of large river/lake basins.
github-actions
bot
added
docs
|
Welcome, new contributor! It appears that this is your first Pull Request. To give credit where it's due, we ask that you add your information to the
Please make sure you've read our contributing guide. We look forward to reviewing your Pull Request shortly ✨ |
for more information, see https://pre-commit.ci
|
There was a problem hiding this comment.
If the flow_index is not in the literature, I suggest to remove it from this PR and keep it in your project scripts.
Once this PR is merged in, I think it would be interesting to look at additional indices that requires other variables like precipitation, as I think it would provide better physical understanding of processes.
The next step will be to write a test for each index to make sure it behaves as expected. You'll see plenty of examples in the test directory.
for more information, see https://pre-commit.ci
Co-authored-by: David Huard <huard.david@ouranos.ca>
Co-authored-by: David Huard <huard.david@ouranos.ca>
faimahsho
force-pushed
the
Watershed-indices
branch
from
1a48fe2
to
70e0174
Compare
faimahsho
force-pushed
the
Watershed-indices
branch
2 times, most recently
from
948fa30
to
0507d4c
Compare
faimahsho
force-pushed
the
Watershed-indices
branch
from
8641c3b
to
bb69dd4
Compare
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Looks good. Please review the changes I just pushed, and add tests for the "indicators" I created.
Note that I added an indexer argument, which lets you pick a season or specific months. Let me know if you believe this is useful or not.
I think the next step would be to have Louise review the finalized PR.
| }, | ||
| "HIGH_FLOW_FREQUENCY": { | ||
| "long_name": "Fréquence des débits élevés", | ||
| "description": "Nombre de jours où le débit est {thresholf_factor} fois supérieur à la médiane.", |
There was a problem hiding this comment.
| "description": "Nombre de jours où le débit est {thresholf_factor} fois supérieur à la médiane.", | |
| "description": "Nombre de jours où le débit est {threshold_factor} fois supérieur à la médiane.", |
faimahsho
force-pushed
the
Watershed-indices
branch
3 times, most recently
from
d8ae74d
to
7beffe9
Compare
…ean over whole period. This part of the analysis can be done on the user-side. Added support for **indexer, allowing more freedom when selecting the period
faimahsho
force-pushed
the
Watershed-indices
branch
from
7beffe9
to
4e086d6
Compare
Co-authored-by: Trevor James Smith <10819524+Zeitsperre@users.noreply.github.com>
for more information, see https://pre-commit.ci
| @declare_units(q="[discharge]") | ||
| def base_flow_index(q: xarray.DataArray, freq: str = "YS") -> xarray.DataArray: | ||
| def base_flow_index(q: xr.DataArray, freq: str = "YS") -> xr.DataArray: |
There was a problem hiding this comment.
What I will bring up was already there before the PR, but it would be good to address this while we're at it.
dischargeis both a variable type defined inunits.py(like "precipitation" & "temperature"), but also a variable name invariables.yml(likepr&tas). I think it should be one or the other- functions in
hydrologyshould use something likedischargeorstreamflowinstead ofq, i.e. we should use a variable name defined invariables.yml. At least I think we usually try to follow this rule? - Speaking of which,
dischargeandstreamflowinvariables.ymlseem to define the same quantity? Maybe we can removedischargefromvariables.ymland keep it only as a variable type, then usestreamflowas a variable name?
Maybe we want a shorter variable name than streamflow? I would advise against q because of quantiles which are often used with xclim, but since we are splitting sdba maybe this will be less an issue in the future.
There was a problem hiding this comment.
I found this : #1431
So it seems the issue is that there is no convention on the name. q is often used, but I agree that its ambiguous.
I'm not sure (1) is a showstopper. But as discharge is not actually used in any indicator, we could indeed remove it from the variables.yml.
@RondeauG how breaking would it be for downstream (xhydro) if we changed all the q inputs to streamflow ? Does that make sense to you ?
There was a problem hiding this comment.
3. Speaking of which, `discharge` and `streamflow` in `variables.yml` seem to define the same quantity? Maybe we can remove `discharge` from `variables.yml` and keep it only as a variable type, then use `streamflow` as a variable name?
This was because discharge was the legacy name used in xclim, but partners in xhydro much preferred streamflow (as discharge had a strong connotation with dams and regulated flow).
However, I agree that streamflow is kinda long. Q is the standard in hydrology for streamflow in m3 s-1, and q for m3 s-1 km-2, but if it's too confusing with quantiles, we could find something else.
As to how breaking this would be for xhydro, this is completely fine for now. None of these indicators have been hard-coded anywhere in the package. The only explicit calls to xclim indicators are for stats and the frequency analysis functions, both of which use da.
There was a problem hiding this comment.
If we sort out the situation with the variables used in the streamflow indices, then the cfcheck in Streamflow class can be removed. See the discussion .
streamflow_flow_indice is a submodule in the xclim indices module to calculate streamflow signatures, representing individual watershed characteristics of large river/lake basins.
Pull Request Checklist:
number) and pull request (:pull:number) has been addedWhat kind of change does this PR introduce?
Does this PR introduce a breaking change?
This can develop unique watershed attributes.
Other information: