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

Overhaul the subdaily disaggregation logic in initialize_atmos.c #240

Closed
3 tasks
jhamman opened this issue Aug 6, 2015 · 10 comments
Closed
3 tasks

Overhaul the subdaily disaggregation logic in initialize_atmos.c #240

jhamman opened this issue Aug 6, 2015 · 10 comments

Comments

@jhamman
Copy link
Member

jhamman commented Aug 6, 2015

When we changed the default timestep units to seconds (#188), we introduced a number of bugs into the subdail disaggregation of forcing variables in initialize_atmos.c. Looking at the code, there is a lot of repeated logic that could be abstracted to individual functions. This will greatly reduce the likelihood of bugs coming from slightly different implementations with the same goal.

I'll take point on this issue and will split the work into three steps:

  • Create functions for the various forms of subdaily disaggregation supported in initialize_atmos.
  • Replace repeated sections of code with these functions
  • Test, test, test.

xref #42 and #188

cc @tbohn: I may need a bit of guidance as I go through this.

@jhamman jhamman self-assigned this Aug 6, 2015
@jhamman jhamman added this to the 5.0 milestone Aug 6, 2015
@bartnijssen
Copy link
Member

Can you point to some examples? The MTCLIM was wrapped with as few changes to that code as possible and I would like to keep it that way. Any VIC code we can of course change.

@tbohn
Copy link
Contributor

tbohn commented Aug 6, 2015

I think Joe is referring to the logic in initialize_atmos.c for
transferring data from the MTCLIM data structures and the forcing data
structures into the atmos data structure.

Ted

On Thu, Aug 6, 2015 at 12:32 PM, Bart Nijssen notifications@github.com
wrote:

Can you point to some examples? The MTCLIM was wrapped with as few changes
to that code as possible and I would like to keep it that way. Any VIC code
we can of course change.


Reply to this email directly or view it on GitHub
#240 (comment).

@tbohn
Copy link
Contributor

tbohn commented Sep 16, 2015

OK, as per @jhamman 's suggestion, I'll take a look at getting this subroutinized.

@jhamman
Copy link
Member Author

jhamman commented Sep 16, 2015

Alright, I think I've fixed some of these issues shown below but will have to rerun this test when I get back to Seattle. A summary of what where I see the subdaily disaggregation (check these boxes when we are satisfied that things are working):

  • Precipitation: looks right, need to plot subdaily subset
  • Air Temp: looks right, need to plot subdaily subset
  • Shortwave: Units problem, needs work
  • Longwave: Units and indexing problem, needs work
  • Density: broken
  • Pressure: broken
  • VP: broken
  • Wind: looks right, need to plot subdaily subset

stehekin_forcings
Figure: Comparison of input forcings for one grid cell (48.1875, -120.6875) in the Stehekin test dataset with the disaggregated hourly forcings for VIC 4.2.b and the develop branch.

Figure current as of January 26, 2015.

@tbohn
Copy link
Contributor

tbohn commented Sep 16, 2015

Thanks @jhamman. Are you actively working on this? What kind of timeframe did you have in mind for this? I can get to this tomorrow, maybe even later today, but I do have some other things to work on that need to be my top priority today.

@jhamman
Copy link
Member Author

jhamman commented Sep 16, 2015

Actually, I also have this plot for a shorter period:

stehekin_forcings2
Figure: Same data as above but for shorter time period and only showing VIC 4.2.b (left) and develop branch (right).

@jhamman
Copy link
Member Author

jhamman commented Sep 16, 2015

@tbohn -

After talking with @bartnijssen, we have come up with another path forward here. The basic idea is to remove the MTCLIM and subdaily disaggregation entirely from the Classic Driver.

  1. Remove MTCLIM
  2. Replace initialize_atmos with vic_force, basically a function to pack the atmos_struct
  3. Create a standalone forcing generator tool based on initialize_atmos that uses the VIC.4.2 C code to do the MTCLIM/disaggregation

This greatly simplifies the Classic Driver implementation but requires us to develop the stand alone tool in the near future.

@tbohn - how do you see this working out?

@tbohn
Copy link
Contributor

tbohn commented Sep 16, 2015

That makes more sense to me. Will the stand-alone tool be based on your python tool? Or will it be in c?

Seems to me that the stand-alone tool will need netcdf i/o (in addition to classic ascii), so maybe python is the way to go?

@bartnijssen
Copy link
Member

I think we should do the standalone tool in two steps:

  1. The existing VIC code with all (most) VIC functionality stripped out. This tool would be able to reproduce exactly the hourly VIC forcings that we had in VIC 4.2. That way we are not making too many changes at once.
  2. Switch to a python-based forcing tool, which may or may not have additional functionality.

Creating (1) would hopefully be straightforward. It would be what VIC produces with the OUTPUT_FORCE option. It'll require a few updates to handle the subhourly timestep.

@jhamman
Copy link
Member Author

jhamman commented Dec 24, 2015

closed as wont fix since we removed sub-daily dis-aggregation all together in #288.

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

No branches or pull requests

3 participants