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

Use area weighted regridding in wflow diagnostic #1643

Merged
merged 2 commits into from May 14, 2020

Conversation

bouweandela
Copy link
Member

@bouweandela bouweandela commented May 4, 2020

This uses the area weighted regridder for the wflow diagnostic (instead of linear).


Tasks

  • Create an issue to discuss what you are going to do, if you haven't done so already (and add the link at the bottom)
  • Give this pull request a descriptive title that can be used as a one line summary in a changelog
  • Make sure your code is composed of functions of no more than 50 lines and uses meaningful names for variables
  • Circle/CI tests pass. Status can be seen below your pull request. If the tests are failing, click the link to find out why.
  • Preferably Codacy code quality checks pass, however a few remaining hard to solve Codacy issues are still acceptable. Status can be seen below your pull request. If there is an error, click the link to find out why. If you suspect Codacy may be wrong, please ask by commenting.
  • Please use yamllint to check that your YAML files do not contain mistakes
  • Update documentation for the recipe to the doc/sphinx/source/recipes folder

If you need help with any of the tasks above, please do not hesitate to ask by commenting in the issue or pull request.


@Peter9192 Peter9192 self-requested a review May 7, 2020
@Peter9192
Copy link
Contributor

Peter9192 commented May 7, 2020

Hey @bouweandela ,
Although I think the implementation looks straightforward and works, the performance of the area-weighted regridder is not great. Long run times and memory bottlenecks were the reason to try and make regridding lazy in the first place.

I've done some tests for the Rhine

  • With preprocessor / linear:
real	8m24.848s
user	33m33.133s
sys	7m0.583s
  • Without preprocessor / linear:
real	31m57.236s
user	41m59.245s
sys	28m58.079s
  • With preprocessor / area-weighted
Not finished within time limit (1h) (Running a longer job now, I'll update this)
  • Without preprocessor / area-weighted
Not finished within time limit (1h)

Simulatenously, I'm running our preprocessor notebook for all catchments. For smaller catchments, the area-weighted runs were handled quite smoothly when I re-introduced the preprocessor.

Thus, removing the extract-region preprocessor seems to deteriorate the performance, even though it saves writing the intermediate netCDF files. I guess we're still loading the entire globe into memory for each time block (and weighting it!?), which makes the source chunks the main bottleneck. Perhaps, rather than discarding the preprocessor altogether, we could move it to the diagnostic, so that a only the relevant lat/lon part of the cube is realized for the regridding?

Furthermore, I think we need to re-evaluate whether the huge cost of this regridder is actually worthwhile. But it's nice that we now have an easy option to switch back to linear :-)

@bouweandela
Copy link
Member Author

bouweandela commented May 7, 2020

Hi Peter,

Moving extract_region to the diagnostic will probably not help, because the runtime increase comes from the much larger amount of data that needs to be preprocessed/saved before the diagnostic can start. However, half an hour looks like quite an acceptable runtime to me. The problem is that area weighted regridding does not work if you apply the extract_region preprocessor first. On my laptop it took about 2 hours to process a single year with area weighted regridding, but that was before I increased the chunk size by a factor of 5.

@Peter9192
Copy link
Contributor

Peter9192 commented May 7, 2020

The problem is that area weighted regridding does not work if you apply the extract_region preprocessor first.

Why is that?

Note that the half hour is for linear regridding, 10 years.
It took appr 45 minutes to run 1 year for the Rhine with area-weighted regridding, but I did re-introduce the preprocessor. The 10-year run has now been running for 3.5 hours.

@Peter9192
Copy link
Contributor

Peter9192 commented May 7, 2020

because the runtime increase comes from the much larger amount of data that needs to be preprocessed/saved before the diagnostic can start.

No, actually the preprocessor is still quite quick. The runtime increase seems to come from the diagnostic.

This is for 10-year runs for the Rhine with linear regridding

  • with preprocessor: Preprocessing steps took 2:11 mins; Diagnostic took 6:08 minutes
  • without preprocessor: Preprocessing steps took 3:21 mins; Diagnostic took 28:33 minutes

@bouweandela
Copy link
Member Author

bouweandela commented May 7, 2020

I did re-introduce the preprocessor.

This doesn't work if the catchment crosses the prime meridian (e.g. try the Meuse, as in the recipe here), because CMOR requires a longitude coordinate between 0 and 360 and then iris does no longer recognize that the coordinate and bounds are contiguous, which is apparently required for the area-weighted regridder.

@bouweandela
Copy link
Member Author

bouweandela commented May 7, 2020

No, actually the preprocessor is still quite quick.

I guess it depends on the hardware then, I got different results when running on my laptop, but that has only two cores.

Copy link
Contributor

@Peter9192 Peter9192 left a comment

I can confirm that it works and at reasonable speed now. Did you want to add a note somewhere about crossing this prime meridian, or do you think it's okay like this?

@bouweandela
Copy link
Member Author

bouweandela commented May 14, 2020

Let's leave it for now.

@bouweandela bouweandela merged commit ed209ca into master May 14, 2020
6 checks passed
@bouweandela bouweandela deleted the wflow-area-weighted branch May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants