CEA-MetroCarac / fitspy

Generic tool dedicated to fit spectra in python
GNU General Public License v3.0
5 stars 0 forks source link

documentation and examples improvements #10

Closed maurov closed 8 months ago

maurov commented 10 months ago

As follow-up to my comments in JOSS review I propose to improve the documentation and examples by taking into account the following points (no particular order/priority and the list is not exhaustive nor complete):

Please, no need to reply point-by-point here, as these are ideas only (not mandatory improvements). If you want to take them into account, I propose to commit your changes to the code/documentation/examples directly into the repository master branch.

patquem commented 9 months ago

Baseline: how to fit the baseline only?

The term baseline is not compatible with the term fit (unlike background which is part of the fit). Indeed, the baseline is defined from a set of points specified by the user. Hoping that the documentation on this matter here helps to better understand the differences between these two approaches.

Baseline/GUI: the "Auto" button adds always a linear baseline, even if the polynomial is selected

I do not observe such behavior. The resulting baseline from the piecewise or polynomial approximation, which appears in green in the figure, takes into account the chosen approximation quite well.

What is the difference between Baseline and "BKG model" in the Peaks section?

See documentation here.

Fitting the background before the peaks

Fitting the background before would require isolating portions of the signal without peaks so that the background is not affected by them, which is not provided in Fitspy. Additionally, in practice, some individuals prefer the background to be fitted simultaneously with the peaks as long as the background model has a physical meaning. The recently given option for users to provide their own background model aligns with this approach.

Multiple X-range regions

Difficult to implement in the current Fitspy context. Nonethless, I'll keep the idea for later. In the meantime, the solution is to process n-portions of the signal according to n-models (which has the advantage of having fewer peaks to fit and therefore probably gains in terms of computation time)

Baseline should be clearly visible in the fits

Added with the related activation key in the Figure settings. commit 80bc760

ASCII input format of the 2D data should be clearly specified

I hope that this section in the documentation clarifies this aspect.

In ex3_gui_2d_maps.py the data are shown, not the fits

This example uses appli.auto_eval(), which adress only the first spectrum. All the 4220 loaded spectra could be processed with appli.auto_eval_all() but this will take much more CPU time. I will add a comment in the example.

Normalization to 100

The people from Raman with whom I worked at the beginning of Fitspy were accustomed to a normalization to 100 with their commercial software.

Thank you very much for your feedback. Hoping that the provided answers meet your satisfaction.

maurov commented 8 months ago

@patquem thanks for your detailed reply and the improved documentation. It is fine for me now.