Possible bug with ~/model/ed/R/write.configs.ed.R

[braczka]

@serbinsh @dlebauer I think I found a problem with the way that 'write.configs.ed.R' converts the leaf_respiration_rate_m2 to dark respiration factor. In order to convert: the code does the following 1) re-scales both the leaf_respiration_rate_m2 and Vcmax from 25C to 15C 2) then dark_resp_factor= leaf_resp@15C/Vcmax@15C.

However, it seems the code rescales Vcmax from 25 C two times, for example starting at line 53.

 if('Vcmax' %in% names(trait.samples)) {
    vcmax <- trait.samples[['Vcmax']]
    trait.samples[['Vcmax']] <- arrhenius.scaling(vcmax, old.temp = 25, new.temp = 15)
  }

  ## Convert leaf_respiration_rate_m2 to dark_resp_factor
  if('leaf_respiration_rate_m2' %in% names(trait.samples)) {
    leaf_resp = trait.samples[['leaf_respiration_rate_m2']]
    vcmax <- trait.samples[['Vcmax']]

    ## First scale variables to 15 degC
    trait.samples[['leaf_respiration_rate_m2']] <- 
      arrhenius.scaling(leaf_resp, old.temp = 25, new.temp = 15)
    vcmax_15 <- arrhenius.scaling(vcmax, old.temp = 25, new.temp = 15)

    ## Calculate dark_resp_factor -- Will be depreciated when moving from older versions of ED2
    trait.samples[['dark_respiration_factor']] < trait.samples[['leaf_respiration_rate_m2']]/
      vcmax_15

It seems that Vcmax has already been re-scaled to 15C even before it enters into the dark respiration factor loop, because the code rescales the Vcmax parameter anyway in the previous section of code. I tested this out and it seems to be the case. For example my median posterior value for leaf respiration rate from the meta-analysis is 2.433. If I convert this by hand correctly, assuming Vcmax@25C=79.357, then I get a dark respiration factor of 0.0306. However, if I rescale Vcmax twice, I get a dark respiration factor of 0.0435, which is exactly what the code is giving me. Does my reasoning make sense or am I way off on this? This might explain why I have been getting 'high' dark respiration factor values from PECAN, that don't work with my simulation. Not sure if the code has recently switched to using Rd0 only, but I am still using dark respiration factor.

[braczka]

@serbinsh Sorry if you warned me about this previously and I neglected to make the proper changes... sometimes it takes a while for things to register for me ;-)

[serbinsh]

Brett,

It appears you are correct. This is likely a result of removing and adding back DRF into the PEcAn code. It seems, as you point out, that we convert Vc25 to Vc15 but later when using Vc to convert Rd into DRF we take the Vc15 from the trait samples and do another conversion which is incorrect.

You and I may have discussed this previously but in any event I think you should get rid of the second conversion of Vc and just use the Vcmax from the trait samples, as long as it is converted to Vc15 previously and added back to the dataframe. Does this make sense? Would you like me to make the change in the code?

[braczka]

Shawn,

A quick note: it seems that this 'double' re-scaling of Vcmax only occurs if Vcmax is being analyzed in the sensitivity analysis as well. If Vcmax is not being analyzed I think it only re-scales Vcmax once within the dark respiration loop, and so the bug doesn't show up.

This is kind of good news to find this bug, and should provide me with much better growth dynamics. I am testing this right now. I am going to make a quick change to my local version of PECAN and keep pushing with my analysis. If you wanted to submit the changes to github go ahead. I am a little confused about how to push the changes to github without updating my entire local version of pecan (which I am trying to keep stable at this point, unless it is something I really need). I know we had talked about creating a branch on github so I could always keep a copy of my local version of PeCAN, but I found the github instructions for creating a branch confusing.

[serbinsh]

Brett,

Ok, I will take care of updating PEcAn to fix this bug.

[dlebauer]

@serbinsh it was a pretty simple fix. The bug would have been caught except that the tests were commented out! https://github.com/PecanProject/pecan/blob/master/models/ed/inst/tests/test.write.configs.ed.R

[rykelly]

Just started using Pecan again and it seems like this change (fe65518bebe3be0546a7a4eea4a015635d671fab) left vcmax_15 referenced on L68 but undefined. I assume just needs this line re-inserted:

vcmax_15 <- arrhenius.scaling(vcmax, old.temp = 25, new.temp = 15)

But I'll leave it to you guys since I didn't follow this discussion.

[mdietze]

That seems correct to me

> On Nov 7, 2014, at 11:58 AM, ryankelly-uiuc notifications@github.com wrote: > > Just started using Pecan again and it seems like this change (fe65518) left vcmax_15 referenced on L68 but undefined. I assume just needs this line re-inserted: > > vcmax_15 <- arrhenius.scaling(vcmax, old.temp = 25, new.temp = 15) > But I'll leave it to you guys since I didn't follow this discussion. > > — > Reply to this email directly or view it on GitHub.

[dlebauer]

I fixed this in 0919270ad9ccb592a28067a4eb66fd8dd77af54c

Sorry I didn't make a pull request - I intended to but had the wrong window open when I made the change.