unit conversions

[wwieder]

related to closed issue #1, but it doesn't look like ANPP values are converted if moved from the location to profile tab. Noticed for KNZ productivity data (link below), but also may be an issue for LUQ elevation gradient, and maybe others? Are these location variables being converted in the first place? If so, what needs to be done so it also occurs when moved to the location tab of KEY2? https://drive.google.com/drive/folders/1KC7Qc-4l9aRg-M081d51-6T4yBudT8b6?usp=sharing

[srearl]

Right, we have been isolating the location and profile aspects of the data, which is a good for some things but presents problems such as this for others. I have reworked the package such that any variable will be converted based on the units-conversion-parameters we established regardless of its location in the key file. You can see the output of the revised package on your KNZ data in my testing folder. Have a look and let me know if that is the output you would expect as well. I am going to leave this on the development branch for the time being, and will merge it into master only after we have done a bit more testing.

devtools::install_github("srearl/soilHarmonization", ref = "global-units-conv")

Separate but related, the QC checks are also specific to the location and profile tabs of the key file. Updating that such that a variable is checked against QC params if outside its expected location (e.g., a location var on the profile tab) is quite a bit more involved. That seems less of an issue since we are soft failing but I can work on that as well if we think it is important and/or this becomes a regular occurrence.

CC @piersond

[piersond]

@srearl Ran through a homog today for the first time since this change and found an error in the QC. The conversion for t/ha --> g/m2 is not being applied, as shown here in UMBS_DIRT_Bulk_Den_2004_2009, where the 162 t/ha agb is carried straight through to the homog file, as opposed to the correct converted value of 16200 g/m2. Same story for 'bgb' which requires the same conversion.

The conversion table in data/unitsConversionLocation.rda has the correct factor for the conversion, so it must be hitting some other snag in the code. It also appears that the error may be limited to this specific unit conversion, as all of the other conversions in the UMBS key were correct.

[piersond]

@srearl Looked closer at the QC output and found a clue. It appears this is a keykey error, we've got stocks going in from the keykey drop down, but the conversion table is looking for a flux.

*Edit: As a test, I pasted a flux unit on the keykey location tab for agb and bgb and the conversion worked. I believe we want the stock units for agb and bgb as we have now, so a proper fix would be to change the unit conversion table.

[srearl]

Believe that I am following this and I think we had recognized this as a potential problem earlier but punted. Can we confer so that I am certain as to the correct changes to be made? @wwieder @piersond

[piersond]

I'm free to talk Wed-Friday, whatever works best for y'all. @srearl @wwieder

[wwieder]

let's do thurs, 10 MT- 9 PT?

On Tue, Jul 23, 2019 at 1:27 PM Derek Pierson notifications@github.com wrote:

I'm free to talk Wed-Friday, whatever works best for y'all. @srearl https://github.com/srearl @wwieder https://github.com/wwieder

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/srearl/soilHarmonization/issues/23?email_source=notifications&email_token=AB5IWJDH3FD24DF6D4VS72TQA5LRNA5CNFSM4H2SZSZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2UFUTA#issuecomment-514349644, or mute the thread https://github.com/notifications/unsubscribe-auth/AB5IWJCDZIBBBY5WQNFNOXLQA5LRNANCNFSM4H2SZSZQ .

-- Will Wieder Project Scientist CGD, NCAR 303-497-1352

[srearl]

@piersond - The revised units look good. I re-homog'd the UMBS_DIRT_Bulk_Den_2004_2009 directory that you referenced above - agb & bgb converted nicely. You can see the output here ... that is som_testing_env_another in the data downloads directory in case that link does not resolve.

Everything up to this point has now been merged into Master, but the update to the units is still in a branch fix-biomass-units-conv. This test of your DIRT data looks good but I will wait to merge this update in case we want to do some more testing first. If so,

install_githhub("srearl/soilHarmonization", ref = "fix-biomass-units-conv")

I will close this issue if we are all happy.

cc @wwieder

[wwieder]

Can we calculate lyr_n_stock_calc as we're doing for lyr_c_stock_calc? This should not write over lyr_n_tot_stock, whith is already in the database.

[srearl]

added