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

Workflow step geo -> process -> 02_daily_data: Rscripts to produce daily values for Precipitation and Flow at a given gage #63

Open
8 of 14 tasks
mwdunlap2004 opened this issue Jul 11, 2024 · 10 comments
Assignees

Comments

@mwdunlap2004
Copy link

mwdunlap2004 commented Jul 11, 2024

  • Generate csv of precipitation and flow for a given dataset
    • Usage: run_model raster_met [scenario] [hydrocode of coverage] [tmp dir=auto] geo process 02
    • Ex: /opt/model/meta_model/run_model raster_met PRISM usgs_ws_01634000 auto geo process 02
  • Calls: Rscript hydroimport_daily.R input_path data_source output_path
    • Takes in data path (could be a link) and outputs the daily csv of the data, needs data source to convert nldas2 to daily values
    • Includes the precip_in and precip_mm columns from precip file
    • makes precip_cfs by weighting precip by gage area (added as column area_sqmi in geo / download / 02_obtain_flow
  • Create step 02_daily_data in https://github.com/HARPgroup/meta_model/tree/main/models/raster_met/geo/process
    • Verify that base script (as of 7/18 this successfully takes the observed precip and converts to daily)
  • When hydroimport_daily.R is updated, migrate it to meta_model:
    • Make a local clone of meta_model repo on your deq2 account (or your personal computer)
    • create branch 02_daily_data (or whatver you want to call your branch)
    • copy script hydroimport_daily.R from your local HARP archive repo into meta_model/scripts/precip/
    • Do PR for Connor/Rob to test on main install
@ilonah22 ilonah22 transferred this issue from HARPgroup/HARParchive Jul 11, 2024
@rburghol rburghol changed the title Workflow 4: Rscripts to produce daily values for Precipitation and Flow at a given gage Workflow step geo -> process -> 02_daily_data: Rscripts to produce daily values for Precipitation and Flow at a given gage Jul 12, 2024
@rburghol rburghol changed the title Workflow step geo -> process -> 02_daily_data: Rscripts to produce daily values for Precipitation and Flow at a given gage Workflow step geo -> process -> 03_daily_data: Rscripts to produce daily values for Precipitation and Flow at a given gage Jul 17, 2024
@rburghol rburghol changed the title Workflow step geo -> process -> 03_daily_data: Rscripts to produce daily values for Precipitation and Flow at a given gage Workflow step geo -> process -> 02_daily_data: Rscripts to produce daily values for Precipitation and Flow at a given gage Jul 17, 2024
@rburghol
Copy link
Contributor

Hey @mwdunlap2004 -- I updated this to note that we want to join the flow data at this step in the workflow, also, since the precip data does not know the drainage area for the gage (we are including it in the usgs data download as a column), I think we may as well go ahead and do the area weighting to add a column for precip_cfs here.

I want to hear if you or @COBrogan has reasons not to -- like, it is combining function which we want to avoid :)

@COBrogan
Copy link
Collaborator

@rburghol I have no problem with this. I think this makes sense to me. This way the daily data is just fully assembled at this step, outputting a file that is ready for any further processing and just has ALL daily data that may be relevant to the analysis. It also appears I wrote some code that depended on this step happening anyways.... so I approve!

@mwdunlap2004
Copy link
Author

Are you saying we'd want it as a part of hydroimport? I feel like that might be a lot to include in one function, but we can definitely do it. I did also edit my usgsdata code to add the da column, all it is a column that just repeats the value for every day, but I don't think that will have any negative effects on our code I'll just have to change how we call it when we're doing the cfs.

@mwdunlap2004
Copy link
Author

Is the drainage area something we want to keep in our daily data? Or once we use it do we not need it?

@rburghol
Copy link
Contributor

Cool @mwdunlap2004 - thanks for making that edit to the USGS code - we will want to migrate that code to the meta_model/scripts/river directory as soon as you are ready (or we can do it, but you may benefit from the PR process for that, and then you'll get credit for the code update in github).

I am with you both, @COBrogan we definitely MUST have the joining of flow and precip in a single file as we all agreed on, but I also am with Michael that it breaks our design rule of 1 step, 1 function. It also means that we add a 3rd argument to the script (the path to the USGS file). But, if we decided that we wanted to koin in the USGS data here anyhow (is that what we decided?), then, we should go ahead and do the precip_cfs conversion since all the data is already there and it is a simple mathematical operation.

To me, the alternative is to make 02_daily_data ONLY summarize precip at a daily time scale, then have a separate function like 03_join_precip_flow to add the obs_flow and precip_cfs columns.

@mwdunlap2004
Copy link
Author

I don't know how to add the update to the meta model so I would appreciate if you could do it (and maybe meet next week to talk about it). I'll probably push to harp archive tomorrow, once we've made a decision on how to handle the daily_data step. Also I just noticed that you mention that it includes precip_mm, right now I haven't been including that, I can definitely change the code to include it, especially if we only need it for the daily step, just let me know.

@COBrogan
Copy link
Collaborator

I don't have a strong opinion on this and I think you guys have a good point here. Thus far maintained one step = one function. Breaking it out further is fine by me. At the end of the day, we are striving to make our architecture as consistent as possible. To that end, a 03_join_precip_flow step would help out with that.

@rburghol
Copy link
Contributor

Let's go ahead and create that separate step then, we can always merge the code if we're discontented :).

@mwdunlap2004
Copy link
Author

I've updated the hydroimport and compdata functions to do this, I also added the calculations for precip_cfs to comp data. As well as adding drainage area as a column to our flow data. If we ever change our minds, it's actually a really simple change, but I do think it makes sense to split them up.

@rburghol
Copy link
Contributor

rburghol commented Jul 19, 2024

This is great @mwdunlap2004 thanks! I will wait till Monday to do the code merge into the meta_model repo, so we can review that process with you and let you do it (though I have included the steps to do this in the issue checklist in the body of the issue if you want to take a crack). This is really coming together :)

I created an issue for this #69

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

No branches or pull requests

5 participants