Feature/refactor cesm put data #710
Conversation
vic/drivers/cesm/src/cesm_put_data.c
Outdated
| @@ -68,9 +68,12 @@ vic_cesm_put_data() | |||
| energy_bal_struct energy; | |||
| snow_data_struct snow; | |||
| veg_var_struct veg_var; | |||
| extern double ***out_data; | |||
There was a problem hiding this comment.
put this above with the other externs
vic/drivers/cesm/src/cesm_put_data.c
Outdated
|
|
||
| for (i = 0; i < local_domain.ncells_active; i++) { | ||
| // Zero l2x vars (leave unused fields as MISSING values) | ||
| // NOTE: I think actually we want unused fields to be | ||
| // filled in with zeros, that's what we did previously |
There was a problem hiding this comment.
Align comments. (Run uncrustify)
vic/drivers/cesm/src/cesm_put_data.c
Outdated
| l2x_vic[i].l2x_Fall_fco2_lnd = 0; | ||
| // for now, uncommenting these out | ||
| // because this might have been the source | ||
| // of the floating point error in RFR runs |
There was a problem hiding this comment.
No problem uncommenting these. I don't think we need the comment here.
vic/drivers/cesm/src/cesm_put_data.c
Outdated
| // Note: VIC does not partition its albedo, thus all types are | ||
| // the same value | ||
| // force->NetShortAtmos net shortwave flux (+ down) | ||
| // SWup = force->shortwave[NR] - energy.NetShortAtmos |
There was a problem hiding this comment.
Do we need these last two comments?
vic/drivers/cesm/src/cesm_put_data.c
Outdated
|
|
||
| // snow height, VIC: cm, CESM: m | ||
| // convert to VIC units | ||
| l2x_vic[i].l2x_Sl_snowh = out_data[i][OUT_SNOW_DEPTH][0] / CM_PER_M; |
There was a problem hiding this comment.
are we sure CPL/WRF isn't expecting SWE?
There was a problem hiding this comment.
In ~/wrf/phys/module_ra_cam.F, it's defined as Snow depth over land, water equivalent (m), which is confusing wording but I take it to mean that we should be returning SWE, not snow depth - so I think you're right
vic/drivers/cesm/src/cesm_put_data.c
Outdated
| // sensible heat, VIC: W/m2, CESM: W/m2 | ||
| // TO-DO: check sign in VIC 4 coupling, this is inconsistent with | ||
| // the history file output sign | ||
| l2x_vic[i].l2x_Fall_sen += -1 * out_data[i][OUT_SENSIBLE][0]; |
There was a problem hiding this comment.
take care of this TODO as part of your next round of changes for this PR.
There was a problem hiding this comment.
Yes - it should be negative. In VIC4 we took care of the sign convention in the lnd_comp_mct.F90 routine, but I think we want to do this in the VIC driver now.
vic/drivers/cesm/src/cesm_put_data.c
Outdated
| // so not repeated here | ||
| evap = out_data[i][OUT_EVAP][0] * MM_PER_M; | ||
| l2x_vic[i].l2x_Fall_evap += -1 * evap / | ||
| global_param.dt; |
There was a problem hiding this comment.
do you really need this temporary evap variable?
There was a problem hiding this comment.
I don't think so - just took it out.
|
@jhamman and @bartnijssen, this is ready for a full review. I'm holding off on making corresponding changes for PR #712 since they'll break the Travis tests, so I'll make those in a quick follow up PR after this and #712 have been merged. |
| // adjust sign for CESM sign convention | ||
| l2x_vic[i].l2x_Fall_lwup = -1 * | ||
| (out_data[i][OUT_LWDOWN][0] - | ||
| out_data[i][OUT_LWNET][0]); |
There was a problem hiding this comment.
Do we have a longwave up variable in out_data?
There was a problem hiding this comment.
Nope - we have LWDOWN, LWNET and the band-specific version of LWNET in out_data.
…IC into feature/refactor_cesm_put_data
|
@jhamman thanks. Just added a release notes entry. |
|
@bartnijssen @jhamman this should be ready to merge. |
new tests addedscience test figuresThis PR refactors the
cesm_put_dataroutine in the CESM driver so that the same output data is being written to the coupler fields and the history files, barring a few sign convention differences. It also sets the dustl2xfluxes, dry deposition velocity and MEGAN fluxes to 0, as I don't think we should be passing these with fill values to the coupler (previously in RASM we populated unused fields with zeros).A few remaining to-dos (which we may want to put in a separate PR):