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

Cache fim catfim #631

Closed
wants to merge 6 commits into from
Closed

Cache fim catfim #631

wants to merge 6 commits into from

Conversation

shawncrawley
Copy link
Collaborator

This PR contains all of the latest CatFIM updates made over the last couple of months per FIM team feedback. The biggest and most notable of these updates is the processing and inclusion of the Stage-Based CatFIM "Interval" sublayers.

@TylerSchrag-NOAA
Copy link
Contributor

Thanks Shawn!

Nick - I'm going to add a little to this, and will review/approve when it's ready.

@nickchadwick-noaa
Copy link
Collaborator

Sounds good, thanks for the help Tyler!

…FIM) in order to do precise stage_ft interpolation, and not cause issues with using NWS Station ID as additional unique constraint (in addition to the regular hand_id and feature_id).
@TylerSchrag-NOAA
Copy link
Contributor

Ok, I'm admittedly not super happy about how hacky/redundant some of this had to turn out. - Kind of 3 steps forward, 2 steps back on the whole abstraction of the HAND Processing workflows. But all of CatFIM is [mostly] working now with the new setup. I'll make some specific comments in the code itself.

Generally speaking, I created a 'alternate_hand_preprocessing' folder in the fim-data-prep lambda function, which houses sql files where additional data prep is needed before running HAND. This could theoretically apply to any FIM Config, but at present it's being used for the FIM Configs where we don't utilize the HAND Cache, i.e. AEP FIM and CatFIM, but do want to do some preprocessing/loading, i.e. pull from the Ras2FIM cache.

I shoulda/woulda/coulda thought through these AEP FIM and CatFIM requirements when designing the Cached FIM enhancement, but as a future optimization I would recommend something like:

  1. abstracting HAND processing into its own modular component, that essentially acts like a super simple API. Give it a list of features with flows (or stages), and get back those features with geometry extents. I'd let it handle all the FIM crosswalk stuff as well (the fim_caching_templates sql files I setup in the postprocess_sql lambda are a step in this direction).
  2. Have an abstract aggregator that exists right before that step in the pipelines, that distills all the features of any set of fim_configs down to that single unique list of features and flows (or stages) to pass off to hand_processing.
  3. Adjust the product processing steps of the pipeline to pull from a) the cache, and b) a shared hand_processing output table to get the cumulative fim extents that it needs (it would be good to standardize the fim caches (ras2fim and hand) into a single cache source as well that can play nice in a simple way with this framework).

@@ -0,0 +1,54 @@
DROP TABLE IF EXISTS publish.stage_based_catfim_minor_intervals_job_num;
Copy link
Contributor

Choose a reason for hiding this comment

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

So I'm sure this has to do with the triple-bracket code you added to viz_classes.py at the top for sql_replace... but I'm getting job_num in the sql_replace dictionary instead of {job_num}.

I just removed the brackets in the sql code for now to get it working... but you'll want to fix that. I'd recommend adding some code to initialize_pipeline to insert those template sql items in the product config into the sql_replace dictionary at the very beginning of a pipeline, so they don't need to be added in fim-data-prep and postprocess-sql separately (that would also be a good opportunity to add my new fim_caching sql_replace params to that same logic in initialize pipeline, per my sloppy code in postprocess_sql, sorry).

@nickchadwick-noaa nickchadwick-noaa added this to the V2.1.x milestone Mar 27, 2024
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.

3 participants