Repository navigation
fix(pedexport): integer layout for WOMBAT, add HIBLUP format - #13
Conversation
pedexport(software = "wombat") in 1.10.0 wrote character IDs based on the incorrect assumption that WOMBAT accepts alphanumeric codes and recodes them internally. Real WOMBAT rejects such files: the manual (section 6.3) requires three integer variables with codes in 0..2147483647, offspring codes numerically larger than either parent, and unknown parents coded 0. The wombat format now shares the integer layout with blupf90 (including the xref attribute and <file>.xref mapping file). New software = "hiblup" format for HIBLUP's --pedigree file: character IDs, no header by default, missing parents coded "0". Also document that ASReml-SA silently ignores !SKIP on the pedigree file line unless it is placed immediately after the file name. All changes verified end-to-end against real binaries (ASReml 4.3, WOMBAT 26-05-2025, renumf90, HIBLUP) on a simulated 1500-individual pedigree: all four programs converge on pedexport() output, variance estimates agree (Va 0.99-1.00 vs truth 1.0), EBV correlations >= 0.9999, and renumf90/WOMBAT inbreeding coefficients match visPedigree's f. Bump version to 1.10.1.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f85780e335
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| missing_int <- suppressWarnings(as.integer(missing)) | ||
| if (!is.numeric(missing) || length(missing) != 1L || is.na(missing) || | ||
| is.na(missing_int) || missing_int != missing) { | ||
| stop("For numeric formats (blupf90, mtdfreml, dmu, numeric), ", | ||
| stop("For numeric formats (blupf90, wombat, mtdfreml, dmu, numeric), ", | ||
| "'missing' must be a single integer value.", call. = FALSE) |
There was a problem hiding this comment.
When software = "wombat" is combined with a custom integer such as missing = -99L, this generic numeric validation accepts it and .pedexport_num() replaces every unknown parent with that value. The resulting file violates WOMBAT's stated requirement that unknown parents be coded as 0, so an explicitly supported argument can produce an unusable or misinterpreted WOMBAT pedigree; reject nonzero missing values for this format.
Useful? React with 👍 / 👎.
WOMBAT manual section 6.3 requires unknown parents to be coded 0; a custom nonzero 'missing' symbol would produce a pedigree file that WOMBAT cannot interpret. Reject nonzero values explicitly. Addresses Codex review on PR #13.
|
Codex review addressed in 2556620: |
1 participant
Summary
Systematic end-to-end testing of
pedexport()(new in v1.10.0) against the real binaries (ASReml 4.3, WOMBAT 26-05-2025, renumf90/BLUPF90, HIBLUP) revealed one genuine bug and motivated one addition:Bug fix —
software = "wombat": v1.10.0 wrote character IDs, assuming WOMBAT accepts alphanumeric codes and recodes them internally. Real WOMBAT rejects such files (ERROR: reading data fileon the first record). The WOMBAT manual §6.3 requires three integer variables, codes in 0..2147483647, offspring codes numerically larger than either parent, unknown parents coded0, ascending order recommended. Thewombatformat now shares the integer layout withblupf90(including thexrefattribute and<file>.xrefmapping file), satisfying all of these by construction.New format —
software = "hiblup": character IDs, no header by default, missing parents coded"0"— ready for HIBLUP's--pedigreefile.Documentation: ASReml-SA silently ignores
!SKIPon the pedigree file line unless it is placed immediately after the file name (empirically verified: trailing!SKIP 1after!MAKEreads the header as 3 phantom individuals; leading!SKIP 1works). The roxygen docs now state the required placement.Verification
test-pedexport.R(150 assertions) + newtest-pedexport-extra.R(~500 assertions: format invariants across all built-in datasets and formats, file round-trips throughfread+tidyped, xref bijection, custom missing symbols,addnum/addgen = FALSEreconstruction, 48.5k-individual scale). Full suite passes viatestthat::test_local()(FAIL 0).pedexport()output directly and converging:asreml(header,!SKIP 1 !ALPHA !MAKE)wombat(new integer layout)blupf90hiblup(new format)EBV correlations between any pair of backends ≥ 0.9999; cor(EBV, simulated true breeding values) = 0.71 (consistent with h² = 0.33).
Notes
software = "wombat"is intentional (the 1.10.0 output was unusable with real WOMBAT); 1.10.0 was never submitted to CRAN, so no CRAN users are affected.man/pedexport.Rdregenerated with roxygen2.