Closed dcianicnr83 closed 1 month ago
Here is the permalink: https://github.com/ecmwf-projects/c3s2-eqc-quality-assessment/blob/abde6d129db83066a4508630892ac251e5311697/Satellite_ECVs/Ocean_ECVs/satellite_satellite-sea-surface-temperature_trend-assessment_q02.ipynb
Please make sure that the notebook name is correct.
If you need to make modifications (e.g., to address the reviewer's comment), please share here the new version of the notebook and I will provide a new permalink.
Let me know when the notebook is accepted, I will merge the PR into main.
Hi @dcianicnr83,
I'm caching this notebook on the new VM, but it looks like there's a bug. Specifically, I think one of the cached function has been modified without invalidating the cache. This is the function in the template I've provided and cached on the old VM:
@cacholote.cacheable
def compute_mann_kendall_trend(
collection_id,
request,
chunks,
year_start,
year_stop,
seasonal,
open_mfdataset_kwargs,
**mann_kendall_kwargs,
):
dataarrays = []
for year in tqdm.tqdm(range(year_start, year_stop + 1), desc="annual"):
requests = download.update_request_date(
request, start=f"{year-1}-12", stop=f"{year}-11", stringify_dates=True
)
ds = download.download_and_transform(
collection_id=collection_id,
requests=requests,
chunks=chunks,
transform_chunks=False,
transform_func=compute_low_resolution,
transform_func_kwargs={"freq": "Q-DEC" if seasonal else "MS"},
**open_mfdataset_kwargs,
)
dataarrays.append(rechunk(ds["analysed_sst"]))
da = xr.concat(dataarrays, "time")
if seasonal:
ds = da.groupby("time.season").map(_mann_kendall, **mann_kendall_kwargs)
ds["trend"].attrs["units"] = f"{da.attrs['units']}/year"
else:
ds = _mann_kendall(da, **mann_kendall_kwargs)
ds["trend"].attrs["units"] = f"{da.attrs['units']}/month"
return rechunk(ds)
And this is the function you have in your notebook:
@cacholote.cacheable
def compute_mann_kendall_trend(
collection_id,
request,
chunks,
year_start,
year_stop,
seasonal,
open_mfdataset_kwargs,
**mann_kendall_kwargs,
):
dataarrays = []
for year in tqdm.tqdm(range(year_start, year_stop + 1), desc="annual"):
requests = download.update_request_date(
request, start=f"{year-1}-12", stop=f"{year}-11", stringify_dates=True
)
ds = download.download_and_transform(
collection_id=collection_id,
requests=requests,
chunks=chunks,
transform_chunks=False,
transform_func=compute_low_resolution,
transform_func_kwargs={"freq": "Q-DEC" if seasonal else "MS"},
**open_mfdataset_kwargs,
)
dataarrays.append(rechunk(ds["analysed_sst"]))
da = xr.concat(dataarrays, "time")
if seasonal:
ds = da.groupby("time.season").map(_mann_kendall, **mann_kendall_kwargs)
ds["trend"].attrs["units"] = f"{da.attrs['units']}/year"
else:
ds = da.groupby("time.year").map(_mann_kendall, **mann_kendall_kwargs)
ds["trend"].attrs["units"] = f"{da.attrs['units']}/year"
return rechunk(ds)
In theory, the function you wrote should create the dimension "year" when seasonal is None
. Because the function has been modified after caching the data, this is not happening on the old VM.
The bug shows up in this line when you make the plots:
row="season" if "season" in ds.dims else None,
You'd have to use the following if you group by year:
row="year" if "year" in ds.dims else None,
What is the desired result?
If you want to achieve the plots shown in your notebook, then we need to use the version of compute_mann_kendall_trend
in the template.
Hi @malmans2 @dcianicnr83, we wanted to calculate the trend, when not grouped by seasons, on a yearly basis instead of having monthly data. So I would say we have to use the new function we provided within the notebook, if possible. Thanks!!
We can use the new function, no problem. But the first plot is going do be different.
Do you want to show column="product"
and row="year"
?
No we want to keep the first plot with the same structure, averaging on the year dimension.
OK, understood!
Great, thanks @malmans2 👍 :-)
Hi there,
I've revised the template and re-cached the data that you need for this notebook. See: https://github.com/bopen/c3s-eqc-toolbox-template/commit/0ac48b2f0c4f9c7f93548df5a090eecde7694706
The frequency changed from Q-DEC
to QE-DEC
to address a deprecation warning.
This notebook and #69 are now broken on the VM. Could you please align the notebooks with the template, and send me the new notebooks? All you have to do is this:
compute_mann_kendall_trend
from the template to your notebookif not seasonal:
ds = ds.mean("year", keep_attrs=True)
Thanks!
Hi @malmans2, I'm trying to implement what you've adapted and I'm realizing that I may have explained badly what we wanted to achieve in the end - we wanted to carry out the trend on yearly averaged data, which is not the same as doing the yearly-average on the trend calculated on monthly data.. Can we somehow fix? thanks
OK, I think I got it, but that's not the code you implemented in the function. Are you OK with a change along these lines?
- ds = da.groupby("time.year").map(_mann_kendall, **mann_kendall_kwargs)
+ ds = _mann_kendall(da.groupby("time.year").mean(), **mann_kendall_kwargs)
ds["trend"].attrs["units"] = f"{da.attrs['units']}/year"
I'd say yes! Thanks :)
OK, I'll do it and let you know when I'm done. It should be much quicker this way - please don't run this function on the VM until I'm done.
I already shut down the previous notebook, when you want you can go ahead!
All set. Can you try again implementing the latest template? Here is the diff: https://github.com/bopen/c3s-eqc-toolbox-template/commit/697d7cb3e0e29892aea46946eff2ff38e12e8363
Ok, we'll implement the new template also in the first use case and return to you :) Thanks, @malmans2 !
Hi @malmans2 , we worked with @vincenzodetoma to produce a new version of the notebook with the new template. Please find it in attachment. satellite-sea-surface-temperature_trends.zip
Hi @malmans2, here it is the notebook with links fixed! Thanks, Vincenzo satellite_satellite-sea-surface-temperature-ensemble-product_trend-assessment_q02_27062024.ipynb.zip
Hi @malmans2, notebook corrected also here. Thanks! satellite_esacci_gmpe_trends_q2.ipynb.zip
Closed in favour of #199
Data Type
Satellite ECVs - Ocean
Assessment Category
Trend Assessment
Dataset Name
satellite-sea-surface-temperature
Question Number
02
Workflow ID
eqctier3-9a17ec54-2b72-4d3e-a061-d8a98013d917
Zipped Notebook
satellite_esacci_gmpe_trends.ipynb.zip
Environment
Anything else we need to know?
No response