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

add cds to _targets pipeline and create thresholds - contingency plan… #15

Merged
merged 8 commits into from
Apr 9, 2024

Conversation

zackarno
Copy link
Collaborator

@zackarno zackarno commented Apr 5, 2024

data pipeline for AWS bucket ECMWF data seems unstable - were supposed to receive today (April 5), but didn't.

Therefore as contingency plan - considering going back to Copernicus Data Store (CDS) for monitoring. I've added the analysis in to calculate the various necessary thresholds and threshold tables based on historical CDS data.

Hopefully we will get an update/reply on Monday and this won't be necessary so hold off reviewing until we chat then.

@zackarno zackarno requested a review from caldwellst April 5, 2024 21:50
@zackarno
Copy link
Collaborator Author

zackarno commented Apr 8, 2024

@caldwellst You'll notice some minor commits since we chatted this morning. They are just to the notebooks for technical note table outputs. I think it's clear what I did, but:

  • spell fixes in table
  • color changes in table
  • duplicate joint activation rate calculation notebook to run on CDs -- i know this copy paste solution was a bit lazy, but it's really much easier given timing.

None of the changes affect the threshold calcs in the _targets pipeline for review

Copy link

@caldwellst caldwellst left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think the comparison is just lacking the key question, which is historically, would we have triggered in the exam same country-year using MARS as with CDS.

@zackarno
Copy link
Collaborator Author

zackarno commented Apr 9, 2024

@caldwellst & @t-downing - just letting you know that with 697586c I updated the zonal files used for CDs zonal stats to be the same file, but with caribbean islands removed.

This is because the historical data downloaded through CDs API is clipped to the simplified shapefile where this has been removed. It also makes sense because the islands are nowhere near the dry corridor. IMO - doesn't really need any/much review as thresholds barely change (a couple mm if at ALL), but just flagging so your not confused on the last push

@zackarno zackarno requested a review from t-downing April 9, 2024 16:57
@zackarno
Copy link
Collaborator Author

zackarno commented Apr 9, 2024

Thanks so much for the quick review!

Looks good to me. I think the comparison is just lacking the key question, which is historically, would we have triggered in the exam same country-year using MARS as with CDS.

I agree 💯 and have added it as an issue reminder (#17 (comment)). Went ahead and merged w/ out addressing it however to ease the git sequencing i had envisioned.

@zackarno zackarno deleted the process-cds-ecmwf branch February 4, 2025 16:11
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