odea-project / qAlgorithms

GNU General Public License v3.0
6 stars 2 forks source link

-log and -ppm #10

Open ricardo-cunha opened 1 month ago

ricardo-cunha commented 1 month ago

Hi Daniel,

I noticed that the -log seems to be on by default as the log_qBinning_beta.csv is always produced/updated at the .exe directory, returning a warning. Is there a way to cancel it?

How should the -ppm be set and where in the cmd string? Should I use -ppm 5 or -ppm=5? Will it be ignored when profile data is given? I ask this for automation purposes?

Also, Gerrit showed a height property which is not in the qPeaks output. Could you add it?

Thanks for the latest release, it works nicely.

Cheers, Ricardo

dahoehn commented 1 month ago

Hello Ricardo, there is currently no way to actively turn off logging. I will add this functionality once the log is finished, at which point it will be opt-in.

The correct formatting for giving a ppm values is -ppm 5. All arguments are position independent. It will be ignored should you use profile data. Since five is the default, you do not have to supply the -ppm argument if that setting is acceptable for you.

To output the height data, use -e instead of -pp. As of the latest release, this also gives you the span in the retention time dimension. The -e(xtended) option is indended to provide all possible data on a peak, where the normal peak printing provides a more easily navigated table. (I noticed that i forgot to write the height value, an updated release will follow shortly)

Thanks for the feedback, Daniel

dahoehn commented 1 month ago

I updated the release, now it also includes height.

ricardo-cunha commented 1 month ago

Thanks for the fast reply and updated release! Is it possible that the warning "Warning: qAlgorithms is intended for profile spectra. A base uncertainty of 5 ppm is assumed for all supplied centroids" is given even when we set the -ppm and is using the given ppm?

dahoehn commented 1 month ago

Currently yes. I plan to change this system such that a warning is only displayed when profile mode data is read in and the -ppm argument was supplied. If you have suggestions for rewording the warning, i am happy to take them.

ricardo-cunha commented 1 month ago

I would split the warnings. If centroided is given but a ppm value is supplied, you could return only the "Warning: qAlgorithms is intended for profile spectra.". Maybe you can add if that is the case something like "Assuming x ppm of uncertainty for the supplied centroids", where x is the defined user ppm or the default. When profile data is given and also the ppm, just ignore the ppm value without warning. That would be better for automation as an additional step would be needed to figure if data is profile or centroid in order to modify the string for the cmd. I would agree with a warning for centroid data as qAlgorithms is really meant to profile data. Just some thoughts.

dahoehn commented 1 month ago

We already check for profile / centroid data, and the warning message only triggers if centroids are read in. This is decided by the dataset consisting to more than 50% of centroids. I have added the ppm value used as a variable to the output now.

I will leave the message "Notice: the changed centroid certainty will only affect pre-centroided data." in, just to reduce the potential for misunderstandings when using the tool. It now only displays if profile mode data was read in and the -ppm argument was supplied.

There is not need to worry about computational overhead here, the input test is only performed once per file.