[Important update] Design Shared pecotmr QC and ColocBoost Analysis Work#494
Closed
xueweic wants to merge 0 commit into
Closed
[Important update] Design Shared pecotmr QC and ColocBoost Analysis Work#494xueweic wants to merge 0 commit into
xueweic wants to merge 0 commit into
Conversation
xueweic
added a commit
to xueweic/xqtl-protocol
that referenced
this pull request
May 24, 2026
Changed `colocboost_analysis_pipeline` to `colocboost_pipeline`. See PR/494 in StatFunGen/pecotmr (StatFunGen/pecotmr#494) for rationale and detailed changes.
xueweic
added a commit
to xueweic/colocboost
that referenced
this pull request
May 24, 2026
See PR/494 in StatFunGen/pecotmr (StatFunGen/pecotmr#494) for rationale and detailed changes.
gaow
reviewed
May 24, 2026
gaow
reviewed
May 24, 2026
| #' @param LD_data A list containing combined LD variants data that is generated by load_LD_matrix. | ||
| #' @param skip_region A character vector specifying regions to be skipped in the analysis (optional). | ||
| #' Each region should be in the format "chrom:start-end" (e.g., "1:1000000-2000000"). | ||
| #' @param return_LD_mat Logical; if \code{FALSE}, return only harmonized |
Contributor
There was a problem hiding this comment.
should be return_LD_data if this is returning the LD object which can be either LD matrix or genotype matrix?
Contributor
Author
There was a problem hiding this comment.
this is a good point. i did not change this code, but this code seems tricky. In current pipeline, we should handle both Xref and LD matrix. Let me revisit this code and try to modify as the minimal changes and impact for other functions.
gaow
reviewed
May 24, 2026
gaow
reviewed
May 24, 2026
| extract_region_name = NULL, region_name_col = NULL, | ||
| qc_method = c("slalom", "dentist"), | ||
| qc_method = c("slalom", "dentist", "rss_qc", "none"), | ||
| finemapping_method = c("susie_rss", "single_effect", "bayesian_conditional_regression"), |
Contributor
There was a problem hiding this comment.
I dont think this logic is necessary now. We just do susie_rss with EB. But maybe this par Haochen should weigh in
Collaborator
|
I tried to push a fix to this but it closed for some reason. I am fixing this! |
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.
1. Interface Separation
Rationale: Direct ColocBoost users and xQTL protocol users need different entrypoints. Mixing both workflows in one function made the API confusing and encouraged duplicated QC logic.
Key changes:
colocboost_analysis()as the direct user-facing wrapper.colocboost()through...colocboost().colocboost().colocboost_pipeline()as the protocol-oriented wrapper.load_multitask_regional_data().2. RegionalData Conversion Layer
Rationale: Loaded regional data should be converted into reusable analysis inputs without reloading files or duplicating conversion logic.
Key changes:
region_data_to_rss_input()for summary-stat/RSS workflows.region_data_to_ind_input()for individual-level workflows.region_data_to_colocboost_input()for ColocBoost-specific assembly.3. Shared QC Refactor
Rationale: QC logic should be reusable across ColocBoost, SuSiE RSS, and individual-level SuSiE workflows instead of being embedded separately in each pipeline.
Key changes:
qc_individual_data()for individual-level QC.summary_stats_qc()to support combined RSS/ColocBoost summary-stat QC while preserving its historical LD-mismatch QC behavior.4. X_ref-Based Imputation
Rationale: When genotype reference X_ref is available, imputation should use the genotype reference directly rather than deriving LD blocks, because LD block partitioning can alter imputation behavior.
Key changes:
colocboost()call.Important report observation:
5. Protocol Compatibility and Minimal Loader Fixes
Rationale: The implementation should support the uploaded chr21 test data and xQTL-protocol use case while keeping loader changes minimal.
Key changes:
6. Documentation and Tests
Rationale: The new interfaces, QC behavior, and imputation decision need clear user-facing documentation and regression coverage so future changes do not reintroduce ambiguity.
Key changes:
colocboost_analysis()direct-input behavior;colocboost_pipeline()protocol-level behavior;