Skip to content

decoding time and relying on it to compute climato#5

Open
remicousin wants to merge 2 commits intomainfrom
remic/time
Open

decoding time and relying on it to compute climato#5
remicousin wants to merge 2 commits intomainfrom
remic/time

Conversation

@remicousin
Copy link
Contributor

A bit more work to read the data but then can take advantage of xarray time crunching facility.

@remicousin remicousin self-assigned this Nov 2, 2022
@aaron-kaplan
Copy link
Contributor

I shared a function that opens the file and fixes the calendar metadata. I think using a function like that would be preferable to copy-pasting the messy details each time we open a file.

In fact, that function already exists in pingrid.py. https://github.com/iridl/python-maproom-tuto/blob/main/pingrid.py#L769 . Sorry I forgot about that earlier.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By calculating the mean before selecting the month, you're doing 12x more computation than needed.

data.sel(month=int(this_month)).mean()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I actually made another change that takes us closer to the ideal set up: which is that the colorscale goes from min to max across the months and thus doesn't change with months. this is more appropriate typically for such a Maproom so that months can be compared between them.

An ideal set up would probably have decent default values but then let the user give the limits of their colorscale.

@aaron-kaplan
Copy link
Contributor

I guess I never actually reviewed this version of the maproom (there are so many now :-( ). If I'm not mistaken, you're re-calculating the min and max over the entire dataset every time you need to render a single tile. That's very inefficient/slow. We've talked about this before and it seems like there's no better solution in some cases than just hard-coding the min and the max. In the python-maprooms version I made them config options.

I'm not sure there's any point continuing to refine this version right now, since the current wave of training is almost over. We'll clean up the one in python-maprooms and then make a new tutorial out of that.

@remicousin
Copy link
Contributor Author

yes. I mostly wanted to have the right reading of time in there. No need to sweat much more about it.

The guessing of the right range for a map (or a graph actually) is indeed not that a trivial problem. Some Maprooms will offer a wide options of maps to plot with very different domains of definition, so pre-computing (or hard-coding) all possible min/max may not be possible. Probably the best strategy is to have a decent default, and then nice controls for the user to adjust.

Anyway... food for thought...

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

Successfully merging this pull request may close these issues.

2 participants

Comments