-
Notifications
You must be signed in to change notification settings - Fork 34
Add MiAge (Mitotic Age) clock (Fixes PR #134) #185
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
Add MiAge (Mitotic Age) clock (Fixes PR #134) #185
Conversation
- Fix syntax error in model definition (= to :) - Replace MiAgeNonlinearClock with proper MiAgeModel implementation - Implement correct optimization-based approach using scipy - Formula: minimize sum((c + b^(n-1) * d - beta)^2) for each sample - Correct MiAge_parameters.csv with actual CpG IDs (not CpG_1, CpG_2) - Remove unnecessary files (MiAge.csv, MiAge_coefficients.csv) - Add expected test outputs from MethylCIPHER reference data - Update model metadata (year: 2018, proper DOI, output label) Based on Youn & Wang (2018) and MethylCIPHER reference implementation.
- Add MiAgeModel to model_gallery.py imports and model_builders registry - Update expected_model_outputs/MiAge.csv with all 10 test samples - Fixes model registration error preventing MiAge clock usage - Tests passing Updates PR bio-learn#134
5e20ac0 to
7959698
Compare
sarudak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good just a couple issues
biolearn/model.py
Outdated
| "file": "MiAge.csv", | ||
| }, | ||
| }, | ||
| "OrganAgeChronological": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These items are all duplicates of above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dear lord, that copy paste method was busy the other day
biolearn/model.py
Outdated
| float | ||
| Estimated mitotic age (number of cell divisions) | ||
| """ | ||
| from scipy.optimize import minimize_scalar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imports should be at top of file
- Remove duplicate ModelGallery entries that were overwriting existing clocks
|
removed duplicates and put imports at top |
sarudak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary
Fixes and closes PR #134