Repository navigation
Dev demographics - #102
Merged
Merged
Dev demographics#102
Conversation
- Implement `.compute_decrement_pair()` to compute exit/stay outcome counts between consecutive snapshot pairs - Implement `estimate_decrement_rates()` (exported) to pool decrement rates across all snapshot pairs in a panel - Implement `.smooth_rate_curve()` and `smooth_decrement_rates()` to graduate and gap-fill rates via loess/linear interpolation - Implement `compute_service_table()` to chain smoothed decrement rates into a multiple-decrement actuarial service table (lx, Lx, Tx, ex) - Register new global variable bindings in zzz.R - Add generated documentation and NAMESPACE export for `estimate_decrement_rates`
…exp` column to `ex`, update docs, and add tests
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical parsing and data-generation defects, plus unresolved rate and life-table correctness issues, remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds demographic decrement-rate estimation, smoothing, and life-table computations, with tests, documentation, and retirement-date processing.
Changes:
- Adds decrement-rate and service-table APIs.
- Adds demographic regression tests and exports.
- Updates personnel data documentation, generation, and dependencies.
File summaries
| File | Summary |
|---|---|
tests/testthat/test-demographics.R |
Adds demographic regression tests. |
renv.lock |
Updates dependency metadata. |
R/zzz.R |
Registers new data symbols. |
R/demographics.R |
Implements demographic and life-table calculations. |
R/data.R |
Documents retirement-date data. |
NAMESPACE |
Exports new APIs. |
man/smooth_decrement_rates.Rd |
Documents smoothing functionality. |
man/estimate_decrement_rates.Rd |
Documents decrement-rate estimation. |
man/dot-smooth_rate_curve.Rd |
Documents the smoothing helper. |
man/dot-compute_decrement_pair.Rd |
Documents snapshot-pair computation. |
man/compute_service_table.Rd |
Documents service-table computation. |
man/bra_hrmis_personnel.Rd |
Updates personnel dataset documentation. |
data-raw/bra_hrmis.R |
Adds retirement-date processing. |
Review details
Files not reviewed (6)
- man/bra_hrmis_personnel.Rd: Generated file
- man/compute_service_table.Rd: Generated file
- man/dot-compute_decrement_pair.Rd: Generated file
- man/dot-smooth_rate_curve.Rd: Generated file
- man/estimate_decrement_rates.Rd: Generated file
- man/smooth_decrement_rates.Rd: Generated file
Suppressed comments (3)
data-raw/bra_hrmis.R:216
- Filtering out pensioner rows and taking
max(ref_date)records the last non-pensioner snapshot asretirement_date. For someone active in 2019 and pensioner in 2020, this writes 2019, so the new field is systematically before the observed transition. Select the first pensioner snapshot for IDs with a prior non-pensioner record, or rename/document this aslast_active_date.
filter(employment_status != "pensioner") |>
group_by(personnel_id) |>
filter(ref_date == max(ref_date)) |>
data-raw/bra_hrmis.R:259
- This adds
retirement_dateonly to the in-memorypersonnel_tbland updates the documentation, but the committeddata/bra_hrmis_personnel.rdais not part of the PR. Package users do not executedata-rawscripts, sodata(bra_hrmis_personnel)will remain stale and lack this column; regenerate and commit the data artifact (or avoid documenting the new field until it is updated).
personnel_tbl <-
personnel_tbl |>
left_join(retirement_fill_tbl, by = "personnel_id", suffix = c("", "_sim")) |>
mutate(
retirement_date = coalesce(retirement_date, retirement_date_sim)
) |>
select(-retirement_date_sim)
man/bra_hrmis_personnel.Rd:23
- Adding
retirement_datemakes the documented dataset contain 12 variables, but the dataset header at line 8 still says 11. Update that count so the documentation matches the new field list.
- Files reviewed: 6/19 changed files
- Comments generated: 8
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+723
to
+725
| "within each group_cols stratum. the survival probabilities will be smoothed, | ||
| and gap-filled automatically if you set smooth = TRUE, see | ||
| help(smooth_decrement_rates) for methodological details.", |
Comment on lines
+225
to
+226
| retirement_date = ifelse(employment_status != "pensioner", NA, retirement_date), | ||
| retirement_date = as.Date(retirement_date) |
| ### rather than showing up with an explicit status at t1 | ||
| observed_types <- unique(t1_status[[status_col]]) | ||
| observed_types <- observed_types[!is.na(observed_types)] | ||
| outcome_types <- union(observed_types, "non-retirement-exit") |
Comment on lines
+433
to
+434
| fit <- stats::loess(rate ~ age, weights = weight, span = span, degree = 2) | ||
| as.numeric(stats::predict(fit, newdata = data.frame(age = full_ages))) |
Comment on lines
+548
to
+551
| active_dt <- smoothed_dt[, | ||
| .(decrement_rate = pmax(1 - sum(decrement_rate), 0)), | ||
| by = c(age_col, group_cols) | ||
| ] |
| #' \item{race}{Worker's race or broad ethnic classification, where available.} | ||
| #' \item{tribe}{Worker's ethnic or tribal affiliation, where available.} | ||
| #' \item{first_employment_date}{Date the worker first entered the public service.} | ||
| #' \item{retirement_date}{The date of retirement for those whose \code{employment_status} is \code{"pensioner"}} |
Comment on lines
+570
to
+575
| #' This chains \code{estimate_decrement_rates()}'s pooled, age-indexed rates | ||
| #' across \emph{age} -- a different axis from the time-pooling that function | ||
| #' already did. By default the rates are graduated first via | ||
| #' \code{smooth_decrement_rates()}, since the chain below requires a | ||
| #' complete, gapless, reasonably stable \code{qx} curve to produce a sensible | ||
| #' result. |
Comment on lines
+722
to
+725
| "compute_service_table() requires a contiguous (no-gap) age sequence ", | ||
| "within each group_cols stratum. the survival probabilities will be smoothed, | ||
| and gap-filled automatically if you set smooth = TRUE, see | ||
| help(smooth_decrement_rates) for methodological details.", |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added demographics to compute life tables