Retain native units on data load

[krzywon]

This was supposed to be the 7th, and final, PR separating #1559. Due to the size, I am separating it into at least 7 more pull requests. I am using this to track the work as I go.

Anticipated pull requests:

• sasdata package
  • refactor nxsunit - Refactor nxsunit sasdata#13
  • separate current data units from units that were loaded
  • helper methods for scaling data and no longer scale to sasview native units on loading
• sasview package
  • preferences window - Add a preferences window #2167
  • ability to scale data to any compatible units and only scale to sasview native units on fitting
  • data loading preferences
  • plotting preferences

[toqduj]

Looking very good so far. However, there is one usability issue remaining (also because Olivier Tache brought it up): we need to be able to set the input/output units. for Q (+Q Uncertainties) and I (+I Uncertainties).
I'm not sure, but an easy way of setting these might be by adding two sub-menus in the "file" menu, called "Set in/out Q units", and "Set in/out I units". These could contain a list of options, one of which has a checkmark next to it, similar to the example in the attached screenshot. Would that be possible?

[krzywon]

Thanks for reviewing and the suggestion, @toqduj. My original plan was to add the feature you are suggesting, but didn't think it would be useful.

Would extra inputs on the data explorer be helpful or just add clutter? I'm thinking an extra section, just below the load data button, with two text boxes for default I and Q units (if none are found in the data file). I would sanitize the inputs and give a warning if the units aren't recognized. This would only effect the data as-loaded.

Exported data take the current state of the data including units (x_unit and y_unit).

[krzywon]

Here is a quick mock-up of what I'm proposing. Input welcome.

[gonzalezma]

It seems good to me, but if you are planning to allow a completely free input from the user, I guess the parsing and the handling of the units may become difficult. What about just a choice menu with the most usual choices (A-1, nm-1, cm-1, what else?).

[smk78]

How would what is proposed work if the data you want to use, say, for simultaneous fitting, had a mix of Q & I units?

[butlerpd]

It does look visually appealing but I am getting very concerned (I believe I have made that comment before 😄 of the amount of stuff permanently on the GUI. It is a delicate balance between making it easy for expert users to find all the bells and whistles and making it easy for new users to access. I think we should work to minimize the clutter that puts a huge barrier to entry whenever possible.

Being on a zoom call with Adrian Rennie and @ajj they both agreed that this should not be part of the permanent GUI as envisioned. The suggestion was that if the file being read does not have units it should pop up asking for units at that time. Also instructions on how to format the units so they will be properly recognized would be helpful in said popup I guess.

Not sure this is the right answer (one has to think through the various consequences), but I think we need to start asking the question regularly when adding to the main page of the GUI: is this absolutely necessary to add to the main page?

My 2 cents anyway. Should we have a discussion at our fortnightly meeting about GUI structure?

[toqduj]

In light of the discussion, I would still have indicated a "default units" in the menus. It's out of the way but can be adjusted if necessary by expert users. It also then defines the output units in graphs..

If you show a dialog every time you load a .dat file, this would get annoying very quickly.

[rozyczko]

Probably doing something wrong but I got it to fail with a simple conversion:
cyl_400_40.txt dataset, converting Q to nm^{-1} and I to cm^{-1} gave me:

12:14:48 - ERROR: Traceback (most recent call last):
  File "D:\projects\sasview\src\sas\qtgui\Plotting\Plotter.py", line 404, in onUnitsChange
    plot.convert_q_units(self.units.cbX.currentText())
  File "D:\projects\sasview\src\sas\sascalc\dataloader\data_info.py", line 185, in convert_q_units
    self.x_unit = unit
  File "D:\projects\sasview\src\sas\qtgui\Plotting\Plottables.py", line 456, in __setattr__
    object.__setattr__(self, name, value)
  File "D:\projects\sasview\src\sas\sascalc\dataloader\data_info.py", line 117, in x_unit
    self.x = self.x_converter(self.x, x_unit)
  File "D:\projects\sasview\src\sas\sascalc\data_util\nxsunit.py", line 478, in __call__
    raise exc
KeyError: 'n m^{-1} is not a Q'

[rozyczko]

This method is identical to that in Plotter.py. Can it be refactored to PlotterBase?

[butlerpd]

I would agree with @toqduj. Having it as a menu item is probably best. considering most files do not have units, or at least not units that are recognized by SasView, Popping up a dialog every time will become really old -- particularly when loading a folder full of them. In light of the discussion I would alter my suggestion then to: read in units from the file. If none assume the units as SasView always has (no behavior change = minimal surprises/confusion for users). Provide menu item for expert users to be able to change the default units etc.

[butlerpd]

Does this have implications for data converter?

[pkienzle]

Wouldn't it be easiest to modify the reduction program to store the units in the file? This could be as simple as "# key: value" pairs before the column data, which makes it pretty easy to write and parse. With value encoded as JSON it is very flexible. The ascii reader would need to define the expected keys and the layout of the corresponding values.

If you can't modify the reduction code, maybe a file reader that sets the units based on file extension?

Shouldn't display units be a user preference independent of storage units? If you are used to looking at curves with Q as 1/nm or 1/m wouldn't you still want to see those units even if you are using data from a different instrument?

Similarly for model parameters, would a user want to see SLDs in units of 1/m²?

Internal units are fixed by the calculation engine(s). We could carry units with us on every input to the calculation, but that amounts to the same thing just with more bookkeeping.

Note that there are consequences to the choice of internal unit. Some fitting engines use step size in parameter space for derivative estimation and for stopping conditions. If SLDs are expressed as 1/m² then values are going to be in the range of 1e-26, and all reasonable steps will be infinitesimal. Parameter values will ideally be in [0.01, 1000], which gives 6-9 digits of precision on the parameter with the usual 1e-8 step size for numerical derivatives.

[gonzalezma]

I started to try this and found some plotting inconsistencies. Here is what I did:

1. In preferences, set default units on loading as m^{-1} for both Q and I (let default plotting as Plot Q/I as loaded)
2. Load the test dataset cyl_400_20.txt. It does not indicate units, so it should assume a Q range from 0.025 to 0.5 meters and intensities from 126 to 0.001 m^{-1}
3. Quick plot shows correctly the data in that range and with the defined units (m and m^{-1})
4. Sending the data to fitting (with cylinder model) plots the data now in A^{-1} for Q (scale from 2e-12 to 5e-11) and cm^{-1} for I (scale from ~1 to 1e-5), but with the wrong labels (m^{-1}). Note also that the residual plot has different units for Q (A^{-1}).
5. Using quick plot again now shows the same plot as the fitting perspective (in A^{-1} and cm^{-1} and with the wrong labels)

The fitting works fine, giving the expected radius of 20e10 A and 400e10 A (i.e. 20 and 400 m), so the conversion from the user units (e.g. m^{-1}) to the internal units (A^{-1} and cm^{-1}) is correct and this seems to be "only" a plotting issue.

I will keep doing some additional tests.

[gonzalezma]

I'm not sure how the "Plot Q as loaded" is intended to work, but as soon as one deselects this option and tries other units on the plotting, selecting it back does not change the plot units.
If the information above the original units used at loading is available, then the corresponding conversion to these units should be applied. Otherwise, possibly the simplest solution is to add a warning message saying that this option is only applicable when loading the data.

[gonzalezma]

Is the unit conversion in the plot supposed to work with 2D data as well?
Loading the NIST file '33837rear_2D_175_16.5_NIST.dat' and then changing the default units on plotting for Q to nm^{-1}, changes the label but not the scale of Qx and Qy in the image.

[butlerpd]

This is getting too big and complicated. @krzywon says we should rethink. Closing this PR but keeping the branch for further work

[krzywon]

I've started the separation of this PR in sasview and sasdata. I'm reopening this PR as a draft so I can track my separation progress.

[lucas-wilkins]

This gets a star too :)