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

try to make functions robust to overlapping horizons #296

Closed
3 of 4 tasks
dylanbeaudette opened this issue Aug 28, 2023 · 6 comments
Closed
3 of 4 tasks

try to make functions robust to overlapping horizons #296

dylanbeaudette opened this issue Aug 28, 2023 · 6 comments

Comments

@dylanbeaudette
Copy link
Member

dylanbeaudette commented Aug 28, 2023

Either adapt or fail gracefully when a profile contains overlapping horizons.

There are several kinds of horizon overlap:

  • Intentional, perfect overlap as with "B/C" style horizons. Overlapping horizons share the same top and bottom depths.
  • Intentional, partial overlap. I have no practical examples, but perhaps this could be used to describe relative proportions within a complex mixture. Without additional information, there is no way to flag this condition valid.
  • Unintentional overlap: most cases, often a data transcription error.

There is no way to judge intent, but we can differentiate perfect vs. partial overlap.

A proper weighted average or dominant condition requires either assumption of equal contribution (50% B and 50% E) or proportionality.

Affected functions (so far):

  • maybe checkHzDepthLogic(..., byhz = TRUE) (return NA in the overlapOrGap column)
  • fillHzGaps() (this is currently the worst offender)
  • dice() (due to ``fillHzGaps()`)
  • plotSPC() (can't fix here) this will require specific changes and a new issue
@brownag
Copy link
Member

brownag commented Aug 28, 2023

Can you elaborate on what you think is needed for checkHzDepthLogic(..., byhz = TRUE)?
I believe this already works for overlapping horizons, at least with some simple test datasets.

As discussed in #197 (where the byhz argument was added), the "overlap or gap" logic error type is meaningless for "by horizon" evaluation. However it is included in byhz=TRUE and FALSE results so that the same columns are present. It might be better to return NA for valid and overlapOrGap so nobody makes the mistake of doing filtering of whole profiles based on byhz=TRUE results.

@dylanbeaudette
Copy link
Member Author

Still working on the full write-up + examples. In short:

  • checkHzDepthLogic(..., byhz = FALSE) works as expected in the presence of overlap
  • checkHzDepthLogic(..., byhz = TRUE) does not (cannot currently) report on overlap or gaps (per horizon level reporting of depth logic errors in checkHzDepthLogic #197)
  • in retrospect, returning NA in the overlapOrGap column would be a nice reminder to those who haven't been following issues (or have forgotten, like me)

I was exploring some ways in which checkHzDepthLogic(..., byhz = TRUE) could report overlap, but I'm not yet convinced that it is wise to make large changes to such an important function.

More context and examples to follow.

@brownag
Copy link
Member

brownag commented Aug 28, 2023

I was exploring some ways in which checkHzDepthLogic(..., byhz = TRUE) could report overlap, but I'm not yet convinced that it is wise to make large changes to such an important function.

I don't think having byhz=TRUE report overlap would be wise, but I may not understand what output you are expecting.
I think that it should return NA though, and probably should have done that in #197 rather than just making a comment about it not being logical as an aside.

@dylanbeaudette
Copy link
Member Author

dylanbeaudette commented Aug 28, 2023

Note that I agreed with that in bullet point 3 above-- you are correct that NA is the correct result here.

I have no plans to tinker any further with checkHzDepthLogic(). Alternatively, I have a new function in mind that seeks to identify the location and nature of overlap in an SPC.

@dylanbeaudette
Copy link
Member Author

Forgot to tag this issue in c80b0d0, which toggles overlapOrGap to NA when byhz = TRUE.

@dylanbeaudette
Copy link
Member Author

Simple example of new flagOverlappingHz() function.

library(aqp)
## two overlapping horizons
z <- data.frame(
  id = 'SPC',
  top = c(0, 25, 25, 50, 75, 100, 100),
  bottom = c(25, 50, 50, 75, 100, 125, 125)
)

depths(z) <- id ~ top + bottom
z$.overlapFlag <- flagOverlappingHz(z)
plotSPC(z, color = '.overlapFlag')

image

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