search

lgatto / pRoloc

A unifying bioinformatics framework for organelle proteomics
http://lgatto.github.io/pRoloc/
15 stars 13 forks source link

Update machinelearning-functions-PerTurbo.R #146

Closed mgerault closed 1 year ago

mgerault commented 2 years ago

Hello,

I don't know if it's because of the new R version but call max with i like this only return one value; as a consequence all prediction scores from perTurbo are the same for each protein with unknown location. Calling max with apply prevent from this issue. I didn't check but it's maybe the case with other machine learning functions from pRoloc.

lgatto commented 2 years ago

Thank you @mgerault - would have have some small test data for us to reproduce this and possibly add a unit test.

@samWieczorek, could you also look at this (as well as #147) , given that, as far as I can remember, you originally contributed that code.

mgerault commented 2 years ago

@lgatto I tested it with tan2009r1 dataset from pRolocdata package and other like dunkley2006 etc. Also, I compared with knn and svm, the results are totally different from perTurbo algorithm --> checked with plot2D and in fData predictions results.

lgatto commented 2 years ago

Thank you very much - I'll look at it asap.

lgatto commented 2 years ago

@samWieczorek - no comment about this?

lgatto commented 2 years ago

Thank you @mgerault - this PR certainly makes sens. I would probably favour 

rowMax(ans[i, ])

for clarity and not dropping the i sub-setting from the original code.

@samWieczorek - I will be waiting for your input in this PR and especially #147 before merging.

lgatto commented 1 year ago

Thank you @mgerault, I'll merge now and push to Bioc. But I haven't hear from @samWieczorek, and might consider deprecating the PerTurbo functions in the future, unless you use them.

mgerault commented 1 year ago

Hi lgatto,

Honestly It's one of the best algorithm with TAGM so it would be a shame to remove it.

Kind regards

Mgerault

Le mer. 21 sept. 2022, 21:00, Laurent Gatto @.***> a écrit :

Thank you @mgerault https://github.com/mgerault, I'll merge now and push to Bioc. But I haven't hear from @samWieczorek https://github.com/samWieczorek, and might consider deprecating the PerTurbo functions in the future, unless you use them.

— Reply to this email directly, view it on GitHub https://github.com/lgatto/pRoloc/pull/146#issuecomment-1254107077, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQENY3Z3RXVWILUQDEQXJBLV7NLMLANCNFSM5YTKHXEA . You are receiving this because you were mentioned.Message ID: @.***>

lgatto commented 1 year ago

OK then, thank you for letting me know. Will keep it. Thanks again for fixing the bug!