Skip to content

Consider validating sensible key values in epi_archive, valid ops for & performance improvements from nonunique keys #89

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

Closed
brookslogan opened this issue Jun 2, 2022 · 2 comments
Labels
op-semantics Operational semantics; many potentially breaking changes here P2 low priority performance REPL Improved print, errors, etc.

Comments

@brookslogan
Copy link
Contributor

brookslogan commented Jun 2, 2022

Currently, we do not check for distinct key values in epi_archive.

  • User experience-wise: this is convenient but also gives them the opportunity to form unexpected or invalid archives.
  • Semantics: we assume there aren't duplicate key values. $as_of might give the right result if the duplicate keys have duplicate values and we don't want duplicate rows as output, but gives the wrong thing if instead user is trying to take advantage of a particular update-reporting structure which might enable some performance improvements, which we could take further advantage of by being more flexible with the key, described next.
  • Performance: we require a version-search for every key value across the single huge archive DT.
    • If all (geo_value, time_value, otherkey1, ..., otherkeyn) are re-reported in every version --- we are working off of full snapshots in DT --- then we only need to look up the version once, and can key by version alone. But we can't use the unique lookup for as_of here; maybe a rolling join would work & generalize to the next case.
    • If all (geo_value, otherkey1, ..., otherkeyn) are re-reported in every version for time_values version - 1:windowlength, then we could key just by (time_value, version).
    • If we have patch-based reporting and no special guarantees, then we need to have (geo_value, otherkey1, ..., otherkeyn, version) as the key, and there should be no duplicates.
    • (Stratifying the DT into multiple DTs, say, one per geo, might increase the number of lookups required but make them faster.)

Might interact with #87.

@brookslogan brookslogan added P2 low priority performance labels Jun 2, 2022
@brookslogan brookslogan added the REPL Improved print, errors, etc. label Jun 7, 2022
@brookslogan
Copy link
Contributor Author

Performance part is duplicate of part of #76.

@brookslogan
Copy link
Contributor Author

as_epi_archive(tibble(geo_value=1,time_value=1,version=1,value=c(1,1)))
#> Error in `new_epi_archive()` at epiprocess/R/archive.R:516:3:
#> ! `x` must have one row per unique combination of the key variables. If you have additional key
#>   variables other than `geo_value`, `time_value`, and `version`, such as an age group column, please
#>   specify them in `other_keys`. Otherwise, check for duplicate rows and/or conflicting values for the
#>   same measurement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
op-semantics Operational semantics; many potentially breaking changes here P2 low priority performance REPL Improved print, errors, etc.
Projects
None yet
Development

No branches or pull requests

1 participant