Changes to allow finetuning at scale #12
Conversation
peterdays
commented
Oct 24, 2024
•
peterdays
commented
- Updated to python3.11
- Updated to pyproject.toml
- Created conda environment.yml for reproducibility
- Data generation:
- changed strategy of splitting (train, val and test vs train and val)
- added option to pass a custom function to process the reads
- added verbose control to finetune_generator
- added progress bars to data generation
- Finetuning:
- added progress bars
- separated gradient accumulation from evaluation
added environment.yml for conda users; added minor changes in the bam function; minor changes and linting
changed tokenizatoin of all data in one go in the dataloader;
reverted finetunedata
updated to py3.11, changes in training loop
dataset bug fix
major improvements in finetuning loop;
| dmrs["abs_areaStat"] = dmrs["areaStat"].abs() | ||
| dmrs = dmrs.sort_values(by="abs_areaStat", ascending=False) | ||
| dmrs = dmrs.sort_values(by="areaStat", ascending=False) |
There was a problem hiding this comment.
Please keep the values to be sorted by the absolute value of areaStat.
'areaStat' (from the DSS package) has a direction, meaning areaStat < 0 is hypomethylated in the targeted phenotype and vice versa. Therefore, if you choose top-N DMRs based on the value, you only get hypermethylated regions in the targeted phenotype.
| dmrs["abs_diff.Methy"] = dmrs["diff.Methy"].abs() | ||
| dmrs = dmrs.sort_values(by="abs_diff.Methy", ascending=False) | ||
| dmrs = dmrs.sort_values(by="diff.Methy", ascending=False) |
There was a problem hiding this comment.
Same comment as line 229.
| n_mers: int = 3, | ||
| split_ratio: float = 1.0, | ||
| split_ratios: List[float] = [0.8, 0.1, 0.1], |
There was a problem hiding this comment.
I am not 100% sure about this part... Although this is the standard design for machine learning, I prefer to keep the test data set from other samples (not seen during the training time) for bioinformatic applications. Even if sequencing reads are different, they are still related/not independent if the source of the biological sample is the same. This is why the original version only splits data into training and valid.
There was a problem hiding this comment.
Hmm, I see! We should make it flexible for both cases, what do you think?
| @@ -16,18 +16,19 @@ bioRxiv 2023.10.29.564590; doi: https://doi.org/10.1101/2023.10.29.564590 | |||
| ## Installation | |||
| _MethylBERT_ runs most stably with __Python=3.9__ | |||
There was a problem hiding this comment.
If you updated the package to python 3.11, this might need to be changed! Or did you test it with 3.9 as well?
There was a problem hiding this comment.
no, you are right! I've worked with python 3.11 only
| > methylbert | ||
| MethylBERT v2.0.0 | ||
| > methylbert | ||
| MethylBERT v2.0.1 |
There was a problem hiding this comment.
I already updated methylbert to v2.0.1 after fixing some bugs. Please change it to '2.0.2'
| @@ -0,0 +1,40 @@ | |||
| [project] | |||
| name = "methylbert" | |||
| version = "1.0.0" | |||
There was a problem hiding this comment.
This version needs to be fixed!
| methyl_caller: str = "bismark" | ||
| methyl_caller: str = "bismark", | ||
| verbose: int = 2, | ||
| read_extract_sequences_func: Optional[callable] = None |
There was a problem hiding this comment.
Is this an option for your own function?
There was a problem hiding this comment.
Exactly! I find it more user friendly this way, one can have a custom read extraction different from the original one without the need to fork the package
|
|
||
| self.step += 1 | ||
| duration=0 | ||
| global_step_loss = 0 |
There was a problem hiding this comment.
I am a bit confused here. Wouldn't the global_step_loss need to be reset when the gradient accumulation step is achieved? I think this should be under "if (local_step+1) % self._config.gradient_accumulation_steps == 0". Maybe I should add one more line to divide global_step_loss with gradient_accumulation_step before recording the performance in f_perform file.
There was a problem hiding this comment.
You are right, I've only worked with gradient accumulation of 1, so I didn't notice that! It's up to you, don't find it necessary to add that division 🤔
|
Merged to the main branch after fixing some bugs! |
