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

Switch Climate Analysis to Multioperation #3221

Merged
merged 3 commits into from Dec 31, 2019

Conversation

@rajadain
Copy link
Member

rajadain commented Dec 31, 2019

Overview

To reduce Celery overhead and the possibility of unchecked chord_unlock calls, we switch Climate analysis to the multioperation geoprocessing call. This results in a more efficient, faster analysis that is easier to cache.

Connects #2782

Demo

image

Testing Instructions

  • Check out this branch and debug celery $ ./scripts/debugcelery.sh
  • Select a WKAOI (preferably a fresh one that would not have been cached on your local machine) and analyze it
    • Ensure all analyses complete successfully, including Climate
  • Select the same WKAOI on staging
    • Ensure the Climate analysis has the same numbers as your local
  • Also try with drawing a shape, so it doesn't use the cache
    • Ensure it completes successfully
This makes explicit the input and output expected of
the `collect_climate` method.
@rajadain rajadain added the PA DEP label Dec 31, 2019
@rajadain rajadain requested a review from mmcfarland Dec 31, 2019
rajadain added 2 commits Dec 31, 2019
To reduce Celery overhead and the possibility of unchecked
`chord_unlock` calls, we switch Climate analysis to the
multioperation geoprocessing call.

`analyze_climate` and `collect_climate` tasks are merged
into one, to expect the new input. Their collective output
remains the same as before.

The view is considerably simplified. Geoprocessing config
is updated to remove the solo calls to `ppt` and `tmean`,
and to have a single `climate` call that gathers data from
all 24 rasters.
@rajadain rajadain force-pushed the tt/analyze-climate-multioperation branch from 0ed5bcc to a680937 Dec 31, 2019
Copy link
Member

mmcfarland left a comment

Looks good. It's hard to compare performance with local vs staging, but it does seem to be an improvement.

@mmcfarland mmcfarland assigned rajadain and unassigned mmcfarland Dec 31, 2019
@rajadain rajadain merged commit 780edda into develop Dec 31, 2019
2 checks passed
2 checks passed
default Build finished.
Details
model-my-watershed-pull-requests Build #4135 succeeded in 10 min
Details
@rajadain rajadain deleted the tt/analyze-climate-multioperation branch Dec 31, 2019
@rajadain

This comment has been minimized.

Copy link
Member Author

rajadain commented Dec 31, 2019

Thanks for taking a look!

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.