2091 slider option for parameters #3790

Stefan14100 · 2025-12-01T14:54:32Z

Description

An InView widget was added to enable real-time comparison of the model with experimental data as selected parameter values are adjusted with sliders. Once the model is satisfactory, the manually tuned parameters can be sent back to the fitting page for further refinement.

Added

src\sas\qtgui\Perspectives\Fitting\media\inview.rst
src\sas\qtgui\Perspectives\Fitting\UI\InViewWidgetUI.ui
src\sas\qtgui\Perspectives\Fitting\InViewWidget.py
src\sas\qtgui\Perspectives\Fitting\UnitTesting\InViewWidgetTest.py

Modified

src\sas\qtgui\Perspectives\Fitting\UI\FittingWidgetUI.ui
src\sas\qtgui\Perspectives\Fitting\FittingWidget.py

Fixes #2091

How Has This Been Tested?

Installed on Windows 10 using the generated installer. Dummy data imported from the ‘example data’ were used with the sphere and sphere–multishell models. The provided parameters were selected, and their ranges were constrained to finite bounds. No issues were observed during these tests, and the InView widget performed as expected.

Review Checklist:

Documentation

There is nothing that needs documenting
Documentation changes are in this PR
There is an issue open for the documentation (link?)

Installers

There is a chance this will affect the installers, if so
- Windows installer (GH artifact) has been tested (installed and worked) -
- MacOSX installer (GH artifact) has been tested (installed and worked)
- Wheels installer (GH artifact) has been tested (installed and worked)

Licensing

The introduced changes comply with SasView license (BSD 3-Clause)

…e wish to fit, with sliders.

krzywon

Overall, this is a really nice new feature! The ability to send the values to the fit will really help users find their initial parameters.

A few general comments, all of which could be future improvements:

Not allowing inf/-inf for the parameter limits will force users to modify the limits for a large number of models and parameters. Maybe set new limits based on the current value of the parameter, instead?
The value inputs in the InView window can probably be smaller, and with fewer significant digits (you currently use 8 decimals, likely only need 3 at most).
The slider step size depends on the range, not the current parameter value. For example, sliding the bar for the scale property, currently set to 1, with a range of 0 to 10000 will step up to 100 and then down to 0. Adding a step size control input could be helpful for this as well.
If possible, a plot of the residuals (as a sub plot of the main plot) would be useful, but that might be beyond the scope of this PR.
I understand why 2D fits are not allowed for now, but would be nice to have.

A few general code comments that will help future developers maintain this work:

Please add docstrings. https://peps.python.org/pep-0257/
Please add type hints. https://docs.python.org/3/library/typing.html

krzywon · 2025-12-03T17:18:10Z

src/sas/qtgui/Perspectives/Fitting/FittingWidget.py

+        """
+        if self._in_view_widget is None:
+            self._in_view_widget = InViewWidget(parent=self)
+            self._in_view_widget.destroyed.connect(lambda: setattr(self, '_in_view_widget', None))


The connection on line 1694 already does this, but in a cleaner way.

krzywon · 2025-12-03T17:20:02Z

src/sas/qtgui/Perspectives/Fitting/FittingWidget.py

+            def _on_inview_closed():
+                self._in_view_widget = None
+                # Enabling paramters edits
+                self.lstParams.setEnabled(True)
+                self.cbCategory.setEnabled(True)
+                self.cmdFit.setEnabled(True)
+                self.cmdPlot.setEnabled(True)


Create the following class-level method and you can replace the last four lines of this function here and in at least 2 other places.

def toggle_inview_window(): toggle = self._in_view_widget == None # Enabling paramters edits self.lstParams.setEnabled(toggle) self.cbCategory.setEnabled(toggle) self.cmdFit.setEnabled(toggle) self.cmdPlot.setEnabled(toggle)

krzywon · 2025-12-03T17:24:48Z

src/sas/qtgui/Perspectives/Fitting/FittingWidget.py

+                if not np.isfinite(pmin) or not np.isfinite(pmax):
+                    has_infinite_bounds = True
+                    break
+            except Exception:
+                # If anything odd, be conservative and flag
+                has_infinite_bounds = True
+                break
+        if has_infinite_bounds:
+            QtWidgets.QMessageBox.warning(self, "InView",
+                "Some parameters have infinite bounds. Please set appropriate min/max ranges.")
+            # Re-enable controls since we are not opening InView
+            self.lstParams.setEnabled(True)
+            self.cbCategory.setEnabled(True)
+            self.cmdFit.setEnabled(True)
+            self.cmdPlot.setEnabled(True)
+            return


Simplification:

Suggested change

if not np.isfinite(pmin) or not np.isfinite(pmax):

has_infinite_bounds = True

break

except Exception:

# If anything odd, be conservative and flag

has_infinite_bounds = True

break

if has_infinite_bounds:

QtWidgets.QMessageBox.warning(self, "InView",

"Some parameters have infinite bounds. Please set appropriate min/max ranges.")

# Re-enable controls since we are not opening InView

self.lstParams.setEnabled(True)

self.cbCategory.setEnabled(True)

self.cmdFit.setEnabled(True)

self.cmdPlot.setEnabled(True)

return

if not np.isfinite(pmin) or not np.isfinite(pmax):

# Fall through to exception handling, which mimics behavior required here.

raise Exception()

except Exception:

# If anything odd, be conservative and flag

QtWidgets.QMessageBox.warning(self, "InView",

"Some parameters have infinite bounds. Please set appropriate min/max ranges.")

# Re-enable controls since we are not opening InView

self._on_inview_closed()

return

krzywon · 2025-12-03T17:27:14Z

src/sas/qtgui/Perspectives/Fitting/FittingWidget.py

+        self._in_view_widget.setData(self.logic.data)
+
+        # Construct sliders from the Fit Page
+        self._in_view_widget.initFromFitPage(self)


You are creating a new window in this method and the fitpage is already passed to the InViewWidget. Instead, pass the data to __init__() and call both setData() and initFromFitPage() as part of the instantiation, which allows you to remove the arguments from both methods.

krzywon · 2025-12-03T17:29:19Z

src/sas/qtgui/Perspectives/Fitting/InViewWidget.py

+    def setData(self, data):
+        if not isinstance(data, Data1D):
+            return
+        self._has_data = True


This won't be true if an exception is thrown later. Maybe move this to after the try/except block?

krzywon · 2025-12-03T18:47:26Z

src/sas/qtgui/Perspectives/Fitting/InViewWidget.py

+        while layout.count():
+            item = layout.takeAt(0)
+            w = item.widget()
+            if w is not None:
+                w.deleteLater()
+            else:
+                sub = item.layout()
+                if sub is not None:
+                    while sub.count():
+                        child = sub.takeAt(0)
+                        cw = child.widget()
+                        if cw is not None:
+                            cw.deleteLater()
+                    sub.deleteLater()


Your solution will only delete 2 levels of children. In case you need a multi-level approach:

def clear_all_inputs(self): layout = self.sliderBox.layout() self._clear_recursion(layout) def _clear_recursion(self, layout): while layout.count(): item = layout.takeAt(0) w = item.widget() if w is not None: w.deleteLater() else: if item.layout() is not None: self._clear_recursion(item.layout()) def _build_sliders(self, params): self.clear_all_inputs()

krzywon · 2025-12-03T18:55:09Z

src/sas/qtgui/Perspectives/Fitting/InViewWidget.py

+        self._slider_meta = {}
+        steps = 1000
+        for name in params:
+            info = self._param_info[name]


Putting this all into a separate method (called _build_param_slider or similar) would be ideal for a number of reasons (easier to find, more modular, easier to refactor).

krzywon · 2025-12-03T19:04:09Z

src/sas/qtgui/Perspectives/Fitting/InViewWidget.py

+            except Exception:
+                pass


You have a number of bare exceptions that are swallowed. What could cause these errors you're capturing and what should the user and/or developer know if this happens?

krzywon · 2025-12-03T19:09:59Z

src/sas/qtgui/Perspectives/Fitting/media/inview.rst

@@ -0,0 +1,27 @@
+.. _InView_Documentation:


A link to this document from the fitting documentation could be helpful.

krzywon · 2025-12-03T19:10:34Z

src/sas/qtgui/Perspectives/Fitting/media/inview.rst

+Next to the interactive plot, one would see sliders for each parameter that has been previously selected. Once the model is satisfactory, it can be send back to Fitting
+via 'Update fitted parameters'.
+
+.. image:: InViewWindow.png


I think you forgot to include the image files here.

bmaranville · 2025-12-08T17:03:36Z

You could filter out parameters that have infinite limits and not show the sliders for those, and provide an easy way to edit the limits if they want to enable slider controls for a parameter.

Stefan14100 added 4 commits November 16, 2025 12:24

New InView widget was added to interactively change the model that on…

28a83d5

…e wish to fit, with sliders.

fixes linting errors

ecf5847

added unit tests and user docs

6093908

fixes linting errors

9870177

krzywon reviewed Dec 3, 2025

View reviewed changes

butlerpd self-requested a review December 16, 2025 14:26

2091 slider option for parameters #3790

Are you sure you want to change the base?

2091 slider option for parameters #3790

Uh oh!

Conversation

Stefan14100 commented Dec 1, 2025

Description

How Has This Been Tested?

Review Checklist:

Uh oh!

krzywon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bmaranville commented Dec 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants