Opened 6 years ago
Closed 6 years ago
#1188 closed defect (fixed)
fitpage hangs if change model while magnetism is on
Reported by: | richardh | Owned by: | |
---|---|---|---|
Priority: | blocker | Milestone: | SasView 4.2.0 |
Component: | SasView | Keywords: | |
Cc: | Work Package: | SasView Bug Fixing |
Description
In 4.2, load some 2d data, send it to fitpage, select sphere model, turn on magnetic, do compute or fit, then change model to core_shell_sphere, the fitpage will hang with error below, suspect a gui problem.
File "C:\sasview42\sasview\src\sas\sasgui\perspectives\fitting\fitpage.py", line 1258, in _on_select_model
self.get_paste_params(saved_pars)
File "C:\sasview42\sasview\src\sas\sasgui\perspectives\fitting\basepage.py", line 3260, in get_paste_params
check = item[1]
Indexerror: list index out of range
Change History (15)
comment:1 Changed 6 years ago by richardh
comment:2 Changed 6 years ago by smk78
Verified.
Full traceback is:
Traceback (most recent call last): File "D:\SasViewDevelopment\sasview\src\sas\sasgui\perspectives\fitting\fitpage.py", line 89, in on_set_focus flag = check_data_validity(self.data) & (self.model is not None) File "C:\Anaconda2x64\envs\sasview\lib\site-packages\wx-3.0-msw\wx\_core.py", line 16712, in __getattr__ raise PyDeadObjectError(self.attrStr % self._name) wx._core.PyDeadObjectError: The C++ part of the FitPage object has been deleted, attribute access no longer allowed. Traceback (most recent call last): File "D:\SasViewDevelopment\sasview\src\sas\sasgui\perspectives\fitting\fitpage.py", line 89, in on_set_focus flag = check_data_validity(self.data) & (self.model is not None) File "C:\Anaconda2x64\envs\sasview\lib\site-packages\wx-3.0-msw\wx\_core.py", line 16712, in __getattr__ raise PyDeadObjectError(self.attrStr % self._name) wx._core.PyDeadObjectError: The C++ part of the FitPage object has been deleted, attribute access no longer allowed. Traceback (most recent call last): File "D:\SasViewDevelopment\sasview\src\sas\sasgui\perspectives\fitting\fitpage.py", line 1256, in _on_select_model self.get_paste_params(saved_pars) File "D:\SasViewDevelopment\sasview\src\sas\sasgui\perspectives\fitting\basepage.py", line 3260, in get_paste_params check = item[1] IndexError: list index out of range
You seem to be able to do everything else in the program except get input focus back in the FitPage.
comment:3 Changed 6 years ago by richardh
ALSO the normal default magnetic parameters only appear if you change model with magnetism off, else they are all zero.
comment:4 Changed 6 years ago by richardh
AND changing the number of shells in a multiplicity model, or including an S(Q), whilst magnetism is on also does a list index out of range
comment:5 Changed 6 years ago by smk78
Copied from email thread [ESS JIRA] (SASVIEW-1199) magnetism issues in 5.0:
@pkienzle says: The bug comes about because of my foolish decision to use parameter names like M0:theta which are not proper python variables. In this particular case, the parameters in the transfer string uses ';' as a parameter separator, which conflicts. Simplest fix is to use ';' as a separator, and hope that there weren't other reasons to use ':'. (1/2 hr) Better fix is to rename the parameters in sasmodels to use an underscore separator. I suspect that having ':' in the parameter names will mean that we can't use constraint expressions for the magnetic parameters. I believe we should place the magnetic tag at the end of the parameter name (sld_M0, sld_solvent_M0) rather than the beginning, much like we do for polydispersity parameters in sasmodels. (2 hr) Or we could do the fake attribute notation (sld.M0, sld_solvent.M0) like we do in the sasview model wrapper, but this will take longer to implement. (4 hr) In sasview we will need page state to translate the name on reload. Do we care about saved magnetic projects at this point?
comment:6 Changed 6 years ago by smk78
- Priority changed from minor to blocker
comment:7 Changed 6 years ago by smk78
Simple fix is available in https://github.com/SasView/sasview/pull/187
I'm inclined not to pull it so close to release.
comment:8 Changed 6 years ago by Paul Kienzle <pkienzle@…>
comment:9 follow-up: ↓ 10 Changed 6 years ago by smk78
@pkienzle says: Actually, it was much easier to update sasmodels. See sasmodels PR 84. Probably better to not apply the sasview patch [PR187], since there is no good reason to change the separator. And anyway, I should have created a separate PR for fixing the default magnetic parameters. Still doesn't fix the problem with too many magnetic parameters shown.
comment:10 in reply to: ↑ 9 Changed 6 years ago by richardh
Replying to smk78:
@pkienzle says: Actually, it was much easier to update sasmodels. See sasmodels PR 84. Probably better to not apply the sasview patch [PR187], since there is no good reason to change the separator. And anyway, I should have created a separate PR for fixing the default magnetic parameters. Still doesn't fix the problem with too many magnetic parameters shown.
the latter has its own ticket #1189
comment:11 Changed 6 years ago by smk78
I have updated Known Issues in the release notes with the gist of this ticket.
The question facing us is whether to release as we are, or to pull @pkienzle's new sasmodels PR84 first?
comment:12 Changed 6 years ago by Paul Kienzle <pkienzle@…>
comment:13 Changed 6 years ago by smk78
@wojciech says: I’ve just merged PR84 and PR85 to sasmodels. I will do some testing on OSX both for 4.2 and 5.0 and if it doesn’t fail I will move a 0.98 tag to HEAD. If it fails, we can release with 0.98 as it is now. @smk78: Can you independently test 4.2 on Win (after it is built), please?
comment:14 Changed 6 years ago by smk78
- Milestone changed from SasView 4.3.0 to SasView 4.2.0
comment:15 Changed 6 years ago by smk78
- Resolution set to fixed
- Status changed from new to closed
Tested 4.2.0 Build 439 on x64 W10.
With the caveat that the magnetic SLD defaults are all 10-6 even though the GUI says they should be in units of 10-6 anyway (the fix is addressed in #1004 for 4.3.0), merging PR's 84 & 85 seems to have fixed the bug reported in this ticket.
Did some other typical user operations as well. Didn't notice any obvious negative consequences of this fix.
Closing this ticket.
The fitpage may recover if focus is changed elsewhere and then back again.
This was on Win7