search

smithlabcode / methpipe

A pipeline for analyzing DNA methylation data from bisulfite sequencing.
http://smithlabresearch.org/methpipe
67 stars 27 forks source link

[Cleanup TwoStatHMM.*pp] Function declarations include non-const, non-reference primitive types? #138

Closed andrewdavidsmith closed 5 years ago

andrewdavidsmith commented 5 years ago

@xjlizji @bdecato @mengzhou Does anyone know why certain functions (e.g. TransitionPosteriors) in the TwoStateHMM and related classes have parameters that are declared as double but without the const or without a reference? These seem like they don't need to change inside the functions, at least in one example I looked at. So I'm not sure why they are declared the way they are.

mengzhou commented 5 years ago

Looks like they were defined as they are from the very beginning, like this one: https://github.com/smithlabcode/methpipe/blame/master/src/common/TwoStateHMM.cpp#L736

andrewdavidsmith commented 5 years ago

Can one of you fix this? Also, there is an instance variable emission_correction_count and rename the class from TwoStateHMMB to TwoStateHMM, which also means changing a couple lines in hmr.cpp?

andrewdavidsmith commented 5 years ago

@xjlizji Did you take care of this?

xjlizji commented 5 years ago

I fixed the const variable types and removed emission_correction_count. Do you mean we should change all TwoStateHMMB to TwoStateHMM? Just want to make sure.

andrewdavidsmith commented 5 years ago

Yes. I think there is no reason to avoid using the TwoStateHMM and I can't even remember why there was a B version made at this point.

xjlizji commented 5 years ago

Maybe for "Baum-Welch". Now all TwoStateHMMB are changed to TwoStateHMM