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

Point comparisons of NASIS/KSSL v.s. SoilGrids using field/standard depth aggregation and mpspline'd data #140

Closed
brownag opened this issue Aug 20, 2020 · 4 comments
Assignees

Comments

@brownag
Copy link
Member

brownag commented Aug 20, 2020

I finalized several methods discussed over the past couple months and put them into {aqp} and {soilDB} that allow for this comparison.

This implementation needs to be expanded to support multi-attribute splining.

I responded to a TSS request recently that was dealing with SoilGrids data -- which was the basis for the new soilDB::fetchSoilGrids. A while back, I developed a method for shoehorning mpspline2::mpspline 1cm interpolation output into a SoilProfileCollection object with similar attributes to its parent object (implemented now as aqp::spc2mpspline.

https://gist.github.com/brownag/d69b899253eef505e1771e7adbef37ad

image

image

@brownag brownag self-assigned this Aug 20, 2020
@dylanbeaudette
Copy link
Member

dylanbeaudette commented Aug 31, 2020

Some notes for later

Figure out a faster way to test this, or at least make the test conditional.

 Examples with CPU or elapsed time > 5s
                  user system elapsed
   fetchSoilGrids 0.46   0.19   38.76

Add argument to re-scale soil properties to common scales.

  lwr <- transform(lwr,
                   id = sprintf("%s-lwr", id),
                   bdod = bdod / 100,
                   cec = cec / 10,
                   cfvo = cfvo / 10,
                   clay = clay / 10,
                   phh2o = phh2o / 10,
                   sand = sand / 10,
                   silt = silt / 10,
                   soc = soc / 10
  )

Consider argument to return a soil profile to describe each part of the expected variation (lower / upper or Q05 / Q95) and central tendency (mean or Q50). This would result in a 3-member SoilProfileCollection for each queried location.

@brownag
Copy link
Member Author

brownag commented Sep 3, 2020

Figure out a faster way to test this, or at least make the test conditional.

 Examples with CPU or elapsed time > 5s
                  user system elapsed
   fetchSoilGrids 0.46   0.19   38.76

The test is skipped if not "in house" but uses the same code as example that ran nearly 40s. I don't think that we can make a reasonable test [or example] more than one point any faster. I always check with --run-donttest as this is now the default for --as-cranon R 4.0.0+ -- so I was aware of this -- but not that it could take this long. I don't think that the long-running examples are a sticking point for CRAN in terms of elapsed time -- it is expected that some of these might run a long time -- they just need to run without error -- and this will ensure they get into the manual and such.

Thanks for adding the donttest{} as clearly I underestimated how long this example takes -- I think it only took 2-3 seconds the several times I tested it and ran examples. I think I did see one come in over 5s one time and chalked it up to slow internet.
Here is the output when we rundonttest{} (note fetchSoilGrids 10s) --- see issue #124

   Examples with CPU or elapsed time > 5s
                           user system elapsed
   fetchHenry              8.27   0.17   19.17
   us_ss_timeline          7.00   0.53  209.68
   fetchNASISWebReport     2.75   0.13   15.26
   fetchSDA_component      2.81   0.02   28.30
   fetchSDA_spatial        0.64   0.09    8.16
   fetchSCAN               0.67   0.01   13.36
   SDA_spatialQuery        0.53   0.03   13.75
   fetchKSSL               0.39   0.05   15.80
   uncode                  0.22   0.07   13.16
   fetchOSD                0.25   0.01    9.10
   OSDquery                0.17   0.03    5.27
   SDA_query               0.17   0.00    6.98
   fetchSoilGrids          0.12   0.00   10.03
   fetchRaCA               0.04   0.05    7.19
   mapunit_geom_by_ll_bbox 0.01   0.00    8.74

Add argument to re-scale soil properties to common scales.

  lwr <- transform(lwr,
                   id = sprintf("%s-lwr", id),
                   bdod = bdod / 100,
                   cec = cec / 10,
                   cfvo = cfvo / 10,
                   clay = clay / 10,
                   phh2o = phh2o / 10,
                   sand = sand / 10,
                   silt = silt / 10,
                   soc = soc / 10
  )

This is a good idea. Implemented in 497d5cc

Consider argument to return a soil profile to describe each part of the expected variation (lower / upper or Q05 / Q95) and central tendency (mean or Q50). This would result in a 3-member SoilProfileCollection for each queried location.

I think this needs more thought, but is a good idea in principle. My instinct here is that we should maintain the 1:1 relationship between query points and profiles for fetchSoilGrids result. If needed develop helper functions that operate/denormalize the "tidy" results that come out of the fetch functions or other similar data.

Here is an issue I created in aqp; I will prototype the method I described. ncss-tech/aqp#164

I don't think that the multi-profile view is typically a useful for quantiative/statistical analysis -- but rather for visualization. Is this something exclusively for making side-by-side profile plots with a common scale? Or are there other more quantitative applications? Similar types of "denormalization" could be done to view SSURGO/NASIS component data -- and really any sort of soil data in an SPC where ranges are depicted with multiple properties -- but interpretation of observational units or block support becomes murky.

@brownag
Copy link
Member Author

brownag commented Dec 30, 2020

It seems that the SoilGrids example and or test has the possibility of failure (actual errors, not just taking a long time). Sometimes the connection gets reset for unknown reasons causing random builds to fail (e.g. 3 out of 4 architectures will pass, one fails with SSL error)

@brownag
Copy link
Member Author

brownag commented Jan 16, 2021

This might be worth re-opening as a discussion or vignette. But there is no soilDB issue remaining here.

@brownag brownag closed this as completed Jan 16, 2021
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

2 participants