merge dev/deepcsa-integration into main#13
Conversation
There was a problem hiding this comment.
Pull request overview
This PR merges the dev/deepcsa-integration branch into main, focusing on making the regression + plotting pipeline more robust to edge cases seen in deepCSA-derived inputs (e.g., no multivariate terms available, LMEM singular fits, omega element construction, intercept forcing config).
Changes:
- Skip multivariate regression output generation when
multi_rules()leaves no remaining elements/predictors, avoiding downstream plotting errors. - Add handling for LMEM “Singular matrix” linear algebra errors during model fitting.
- Fix omega reader element list construction when
config["elements"]is not provided; adjust intercept forcing logic when config is empty/None.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Removes ps from the lockfile dependency set. |
src/regressions/utils.py |
Updates intercept-selection logic for cases where the config doesn’t specify intercept-0 predictors. |
src/regressions/models.py |
Wraps model fitting to bypass singular-matrix errors for mixed models. |
src/regressions/main.py |
Skips multivariate execution/output when rules leave no viable multivariate terms. |
src/plot/coefplot/main.py |
Minor logging message tweak. |
src/create_input/readers/clonalstructure.py |
Fixes omega element generation when elements are not explicitly provided. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| if not elements or not predictors: | ||
| logger.info("Multivariate analysis not possible: no remaining variables after applying rules") | ||
|
|
||
| else: | ||
| output_dir_multi = os.path.join(output_dir, metric, "multivariate") | ||
| os.makedirs(output_dir_multi, exist_ok = True) | ||
|
|
| # no predictors to force a 0 intercept in config | ||
| if config["predictors_intercept_0"] is None: | ||
| intercept = " + 1" | ||
| return intercept | ||
|
|
||
| # if predictors contain at least one that requires zero intercept forcing, force it | ||
| predictors_intercept_0 = config["predictors_intercept_0"] | ||
| if not isinstance(predictors_intercept_0, list): | ||
| predictors_intercept_0 = list(predictors_intercept_0) |
| else: | ||
| raise | ||
| except Exception as e: | ||
| logger.error(f"Unexpected error for formula {formula}: {str(e)}") |
rochamorro1
left a comment
There was a problem hiding this comment.
It works perfectly, Raquel. Now it just does not plot anything if height = 0, meaning if there are no results for multivariate analyses.
Pull request to merge dev/deepcsa-integration into main with the following main changes: