Skip to content
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

Uniformize heat/hot spell indices #2031

Open
1 task done
coxipi opened this issue Dec 20, 2024 · 3 comments
Open
1 task done

Uniformize heat/hot spell indices #2031

coxipi opened this issue Dec 20, 2024 · 3 comments

Comments

@coxipi
Copy link
Contributor

coxipi commented Dec 20, 2024

Generic Issue

  • xclim version:
  • Python version:
  • Operating System:

Description

  • Currently, hot_spell_max_length has window=1 by default, wherehas hot_spell_total_length and hot_spell_frequency have window=3. I suggest we change this to 3 everywhere, so that hot_spell_XYZ describe the same events, and just a different diagnostic
  • heat_wave_index is just hot_spell_total_length with different defaults. "heat" is usually used when both tasmin,tasmax. We could remove this index since it's redundant? At the very least, I will use hot_spell_total_length for the computation
  • heat_spell_XYZ indicators (in _temperature.py) basically do the same as heat_wave_XYZ, we could keep the latter.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@aulemahal
Copy link
Collaborator

heat_spell_XYZ do some arithmetic over the spell while heat_wave_XYZ do not (the criterion is that all days pass). The difference was kept for three reasons : 1) I needed a different name in PC, 2) I didn't want to break anything, 3) I was in a rush and didn't have time to think.

Thus, I think it's ok to remove the heat_wave_XYZ indices, but we could keep the indicators ?

@coxipi
Copy link
Contributor Author

coxipi commented Dec 20, 2024

Ah right, rolling averages. So heat_wave_XYZ is the bivariate version hot_spell_XYZ , whereas heat_spell_XYZ does its own thing.

Well, heat_wave_XYZ indices could probably be removed by using bivariate_spell_length_statistics and win_reducer = "min" ? A lot of indices could be removed if we follow this logic

My main point was the first two points, I just added the comment about heat_wave after, sorry I read too fast. Similarly, we should not change anything about these points to avoid breaking changes? We can keep heat_wave_index as an artefact I guess, but the fact that heat_spell_XYZ defaults are not coherent is a bit more bothering I find.

@aulemahal
Copy link
Collaborator

I think so indeed. It is the goal of having these feature-full "generic" indices that we can remove duplication and jump straight from the "generic" to the indicators.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants