Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better checks on result object so failed fitting is shown as such. #3079

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

rozyczko
Copy link
Member

Description

For some fitting failures, the result object is either None or an empty list. This causes unhandled behaviour and cryptic messages to the user.

Fixes #2994 (well, not actually fixes it but helps with diagnosis)

How Has This Been Tested?

Local Win10 build

Review Checklist:

[if using the editor, use [x] in place of [ ] to check a box]

Documentation (check at least one)

  • 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)

Licencing (untick if necessary)

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

@krzywon krzywon linked an issue Aug 21, 2024 that may be closed by this pull request
Copy link
Contributor

@krzywon krzywon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds better checks. If my concern regarding the results matrix has no merit, I say the code looks good. I haven't tested the functionality

@@ -1148,7 +1148,7 @@ def showFitResults(self, output_data):
Show bumps convergence plots
"""
self.results_frame.setVisible(True)
if output_data:
if output_data and len(output_data) > 0 and len(output_data[0]) > 0:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way output_data can be a scalar quantity if it has any truthiness?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way output_data can be a scalar quantity if it has any truthiness?

Regardless of the failure/success, output_data should always be a tuple, as passed from FitThread/compute, so checking for the truthiness of it is probably unnecessary but maybe prudent.

@wpotrzebowski wpotrzebowski added the SasView 6.0.0 Required for 6.0.0 release label Aug 21, 2024
@wpotrzebowski
Copy link
Contributor

Tested on Mac and getting error but maybe the one that is expected?

21:42:35 - INFO: --- SasView session started, version 6.0.0b2, 2024 ---
21:42:35 - INFO: Python: 3.11.9 (v3.11.9:de54cf5be3, Apr 2 2024, 07:12:50) [Clang 13.0.0 (clang-1300.0.29.30)]
21:43:38 - INFO: cc -std=c99 -O2 -Wall -fPIC -shared /var/folders/1h/nkglbdgx00nf690grw9xl0j00000gn/T/sas64_sphere_5B77761F_nms7ii9x.c -o /Users/wojciechpotrzebowski/.sasview/compiled_models/sas64_sphere_5B77761F.so -lm
21:43:51 - INFO: cc -std=c99 -O2 -Wall -fPIC -shared /var/folders/1h/nkglbdgx00nf690grw9xl0j00000gn/T/sas64_hayter_msa_044EFDB7_hgnz1n14.c -o /Users/wojciechpotrzebowski/.sasview/compiled_models/sas64_hayter_msa_044EFDB7.so -lm
21:44:49 - INFO: Fitting: Terminated!!! === Steps: 9 of 200 chisq: 3.78e+03 ETA: 2024-08-21 21:44 (2s from now) M1.charge : 1e-06 | M1.concentration_salt: 0.00225 | M1.scale : 1.3 M1.volfraction : 0.0249 |
21:44:49 - INFO: 2024-08-21 21:44:49 === Steps: 9 of 200 chisq: 3.78e+03 ETA: 2024-08-21 21:44 (2s from now) M1.charge : 1e-06 | M1.concentration_salt: 0.00225 | M1.scale : 1.3 M1.volfraction : 0.0249 |
21:44:49 - ERROR: Fitting failed: Traceback (most recent call last): File "sas/qtgui/Perspectives/Fitting/FitThread.py", line 79, in compute File "sas/qtgui/Perspectives/Fitting/FitThread.py", line 19, in map_apply File "sas/qtgui/Perspectives/Fitting/FitThread.py", line 16, in map_getattr File "sas/sascalc/fit/BumpsFitting.py", line 284, in fit AssertionError
21:44:49 - ERROR: === Steps: 9 of 200 chisq: 3.78e+03 ETA: 2024-08-21 21:44 (2s from now) M1.charge : 1e-06 | M1.concentration_salt: 0.00225 | M1.scale : 1.3 M1.volfraction : 0.0249 

@rozyczko
Copy link
Member Author

Tested on Mac and getting error but maybe the one that is expected?

21:42:35 - INFO: --- SasView session started, version 6.0.0b2, 2024 ---
21:42:35 - INFO: Python: 3.11.9 (v3.11.9:de54cf5be3, Apr 2 2024, 07:12:50) [Clang 13.0.0 (clang-1300.0.29.30)]
21:43:38 - INFO: cc -std=c99 -O2 -Wall -fPIC -shared /var/folders/1h/nkglbdgx00nf690grw9xl0j00000gn/T/sas64_sphere_5B77761F_nms7ii9x.c -o /Users/wojciechpotrzebowski/.sasview/compiled_models/sas64_sphere_5B77761F.so -lm
21:43:51 - INFO: cc -std=c99 -O2 -Wall -fPIC -shared /var/folders/1h/nkglbdgx00nf690grw9xl0j00000gn/T/sas64_hayter_msa_044EFDB7_hgnz1n14.c -o /Users/wojciechpotrzebowski/.sasview/compiled_models/sas64_hayter_msa_044EFDB7.so -lm
21:44:49 - INFO: Fitting: Terminated!!! === Steps: 9 of 200 chisq: 3.78e+03 ETA: 2024-08-21 21:44 (2s from now) M1.charge : 1e-06 | M1.concentration_salt: 0.00225 | M1.scale : 1.3 M1.volfraction : 0.0249 |
21:44:49 - INFO: 2024-08-21 21:44:49 === Steps: 9 of 200 chisq: 3.78e+03 ETA: 2024-08-21 21:44 (2s from now) M1.charge : 1e-06 | M1.concentration_salt: 0.00225 | M1.scale : 1.3 M1.volfraction : 0.0249 |
21:44:49 - ERROR: Fitting failed: Traceback (most recent call last): File "sas/qtgui/Perspectives/Fitting/FitThread.py", line 79, in compute File "sas/qtgui/Perspectives/Fitting/FitThread.py", line 19, in map_apply File "sas/qtgui/Perspectives/Fitting/FitThread.py", line 16, in map_getattr File "sas/sascalc/fit/BumpsFitting.py", line 284, in fit AssertionError
21:44:49 - ERROR: === Steps: 9 of 200 chisq: 3.78e+03 ETA: 2024-08-21 21:44 (2s from now) M1.charge : 1e-06 | M1.concentration_salt: 0.00225 | M1.scale : 1.3 M1.volfraction : 0.0249 

I think this is the correct message - something went wrong with the fit and the error message shows it, rather than the previous, cryptic output_data[0][0] error.

@wpotrzebowski
Copy link
Contributor

Ok, in that case, I am merging it.

@wpotrzebowski wpotrzebowski merged commit ba87cd9 into release_6.0.0 Aug 22, 2024
28 checks passed
@wpotrzebowski wpotrzebowski deleted the 2994-fitting-failshangs-on-some-fits branch August 22, 2024 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SasView 6.0.0 Required for 6.0.0 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fitting fails/hangs on some fits
3 participants