ISISNeutronMuon
Instrument profiles prototype
Description of work The most difficult part of the analysis input at the moment is the q-vector definition. This PR creates a list of parameter profiles, which will initialise resolution and q vectors to the same input every time, which should normally correspond to the parameters of a specific neutron instrument.
Comment: At some point this library https://github.com/oerc0122/ResConvLib will solve the problems with instrument parameters. For now, it will be good for the users to have an option of saving their most commonly used settings.
Fixes
- Added an instrument tab for viewing and editing profiles,
- Added a settings file with some initial instrument profiles,
- The job tab has a combo box for instrument selection,
- The resolution and q vector widgets get their initial values from the instrument profile. These can still be changed manually by the user.
- Existing definitions can be changed and new ones can be added by the user. These will be stored in a separate TOML file in ~/.mdanse.
To test Load a trajectory and run a Dynamic Incoherent Structure Factor calculation. Change the instrument profile and run the same analysis again. The resolution and q-vector definitions should correspond to those from the instrument profile. Save a script of the previous analysis. It should contain the correct parameters, without actually referencing the instrument definitions. Change the parameters of an instrument and save changes. Restart MDANSE_GUI and confirm that the changes have been saved. Delete the ~/.mdanse/InstrumentDefinitions.toml and start MDANSE_GUI again. The original parameters should be available again.
Looks good.
Here are a few bugs.
- changing the sample from isotropic to crystal causes a crash.
- changing the technique from QENS to INS causes a crash.
- "e" can be entered into the resolition_fwhm box.
- crash when qvector_type is changed for some qvector types.
- putting a "0.1e" into q_min, q_max or q_step causes a crash.
And some suggestions.
- A reset for specific instruments. For example, if I changed the OSIRIS values, I might want to change it back later. Alternatively, the specific instruments shouldn't be editable.
- Some copy functionality.
Also, I noticed that the width setting is auto-populated depending on the q vector step. For the ideal instrument, I'm wondering if this should be small since in the ideal case when I generated q vectors with |Q| = 10 I would expect them all to have |Q| = 10 not |Q| = $10 \pm 1$.
I tried to implement all the changes you recommended.
- Input checks should prevent the GUI from crashing,
- There are 3 actions in the instrument context menu.
- The instruments supplied by us cannot be deleted or renamed, but they can be edited or the original values can be restored.
- Instruments added by the user cannot be restored, can be renamed and can be deleted.
- q_width is now separate from q_step.
With the latest changes, the following happens:
- The normal starting point is "No instrument", which makes the widgets use the same parameters as always,
- If an instrument is chosen, and then the user makes changes to the instrument settings of this specific instrument, the Action tab will be regenerated to use the new settings. But I guess that it will also lose all the other changes the user may have made to the action inputs.
- If a user pick an instrument, and then makes changes to the analysis inputs on top of it, there is still no one-click way to reapply the instrument settings.
I think that the current implementation will become a problem once the users will try to run the same analysis many times, using the parameters of different instruments. Ideally, the instrument choice should only affect the specific widgets it is related to (instrument resolution, q vectors) and not regenerate the entire action resetting all the inputs.
After the latest changes, only the input fields for q vectors and instrument resolution are affected by changes to the instrument definition. So, it is now possible to configure a job, run it, then pick a different instrument and run the same thing again (that is, after renaming the output file). The GUI is also more responsive now.
Is the current state of this PR acceptable, or do you think that we still need the extra button for reloading the same instrument?
Some issues
Select the instrument e.g. "OSIRIS PG002" then change the qvector type to latticeqvector then back to sphericalqvectors, the instrument settings are not restored.
Some thing I've thought about.
- Changing the the instrument and resolution settings so that they should not be changeable when an instrument is selected. So that only the "no instrument" allows those settings to be changed, this will fix the above issue.
- Instrument selection is greyed out for None neutron scattering job e.g. x-ray static structure factor.
- Update static structure factor so that its settings change with the instrument selection?
- Some instruments you'd do certain experiments with them should we allow the user to use those instrument parameters for that job e.g. should all instrument settings be allowed to do the DCSF job?
I think that the discussion now has moved towards comparing the current functionality of the "instrument profiles" to user expectations. If users want to be stopped from making changes to the job parameters, we can make the interface more restrictive in the next iteration. I would vote for merging the current version so we can start collecting user feedback on the current implementation.