compomics / DeepLC

DeepLC: Retention time prediction for (modified) peptides using Deep Learning.
https://iomics.ugent.be/deeplc
Apache License 2.0
52 stars 18 forks source link

The last of default models is selected as the best one #40

Closed caetera closed 2 years ago

caetera commented 2 years ago

Hello,

I have noticed, that when selecting one of the default models (i.e. --file_model is not provided) the last one in the list is selected as the best one independently of the performance. Here is a piece of debug log (I have included the model name in the output):

2022-03-14 20:05:12 - DEBUG - For full_hc_hela_hf_psms_aligned model got a performance of: 0.28377379963897975
2022-03-14 20:05:12 - DEBUG - For full_hc_PXD005573_mcp model got a performance of: 0.28786216590159314
2022-03-14 20:05:12 - DEBUG - Model with the best performance got selected: {'/home/vgor/.local/lib/python3.8/site-packages/deeplc/mods/full_hc_PXD005573_mcp_cb975cfdd4105f97efa0b3afffe075cc.hdf5': '/home/vgor/.local/lib/python3.8/site-packages/deeplc/mods/full_hc_PXD005573_mcp_cb975cfdd4105f97efa0b3afffe075cc.hdf5'}

Although the performance of full_hc_hela_hf_psms_aligned is better, the last one in the list is selected.

Most likely, the error is this line https://github.com/compomics/DeepLC/blob/18fb8c39e0f80576dee0c1ecff0b6738968f2692/deeplc/deeplc.py#L1194 I believe it should be m_group_name = m_name

Since, I am not very sure that I understand everything correctly, I prefer to open the issue before PR.

The issue was observed with versions 0.1.36 and 1.0.0 (the latest in pip)

RobbinBouwmeester commented 2 years ago

Dear caetera,

That does seem like a mistake. I can have a look at this tomorrow. In the meantime, feel free to open a PR.

Thank you for noticing!

Kind regards,

Robbin

caetera commented 2 years ago

PR created #41

RobbinBouwmeester commented 2 years ago

Great! PR merged. Will release a new version soon.

RobbinBouwmeester commented 2 years ago

Solved in #41 , please see v1.0.1