Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add regridding section #9
Add regridding section #9
Changes from all commits
2f90169
d899b51
8cb845b
ca50ee9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it may be more important to say that "Since our data is now on a latlon grid" we can now subtract to get the difference between this and the UM equivalent data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't doing what the prompt asks for (plotting instead of printing).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be too complicated, but it occurs to me that you might suggest the user tries several values for 'resolution' : A graph of time-required vs. residual error should demonstrate the issues quite nicely.
It definitely is worth simply stating that "the time/accuracy is a tradeoff, which the user needs to decide."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd still like it to clearly explain the existence of a time/accuracy tradeoff somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code needs to go in a code cell.
In the current version, it is rendered in markdown, but we need the actual results below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misprint here too
from testdata_fetching import
(missing -l
)fric_temp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me, this step is currently taking an extremely long time ~1m40, , and about half the VDI memory (~2Gb).
So I think that needs reviewing for practicality.
NB with latest code we can "switch" to C48-sized data, which is over 10x smaller (221k -> 14k points).
This may well be worth doing (we should probably make it standard).
But it will affect the current results + so may require some adjustment to be made (e.g. might then be too fast to demonstrate efficiency changes !)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it looks like this problem is specific to longitude bands and performance is a lot better with latitude bands. I think this might have something to do with ESMF not liking long, thin cells. I've raised an issue here SciTools/iris-esmf-regrid#234 . Unfortunately, when taking the latitude band means there is a lot less visible detail in the resulting plot. I might try experimenting with other variables, but I think this is a reasonable trade off compared to minute long run times or C48 data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since merging this, I've also tested it with the newer C48 data.
Unfortunately, that is possibly too small, since the operations get so quick as to make the measurements less certain.
This op took about 1.6 seconds, instead of ~100.