Add resolution helper - Githubissues

ISISNeutronMuon / MDANSE

MDANSE: Molecular Dynamics Analysis for Neutron Scattering Experiments

https://www.isis.stfc.ac.uk/Pages/MDANSEproject.aspx

GNU General Public License v3.0

19 stars 4 forks source link

Add resolution helper #415

Closed MBartkowiakSTFC closed 2 months ago

MBartkowiakSTFC commented 2 months ago

Description of work Adds a helper dialog for defining the instrument resolution function. It uses FWHM as the input parameter, and allows the user to pick a unit of energy of the FWHM. Also provides a preview of the peak function.

closes #343

Fixes

A new dialog has been added to the InstrumentResolutionWidget.
If the user changes the main GUI input fields manually, the helper dialog is updated too.

To test Please try running the DynamicIncoherentStructureFactor analysis, and change the resolution function type and peak parameters both in the main GUI and in the helper dialog.

ChiCheng45 commented 2 months ago

Looks good, just a few things.

Would it be better to have an apply button similar to the other helper dialogs? In the atom mapping and atom selection dialogs the setting is only updated after an apply button is pressed. I think that will also get around the issue of the helper dialog not updating when the actual setting is changed.

Some of the setting boxes allow strings which are not numbers. Also some general visual issues. See the pictures below.

MBartkowiakSTFC commented 2 months ago

I think that I addressed all the issues other than the matplotlib text overlapping or getting clipped. The problems with matplotlib text labels are mainly a result of bad ratio of the figure dimensions, figure DPI and the Qt widget size; I am not sure how to test for it in the code and correct it if such a problem occurs.

ChiCheng45 commented 2 months ago

A few more things to do to clean it up a bit before merge - I will add this.

With ideal the setting values should be N/A like before.

Ideal peak is not centred and the red line which is used to show the FWHM should not be shown. Also the FWHM and centre options should be disabled like eta.

Text can be entered for the eta value