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

WIP. DONT MERGE. Allow polydisperse parameters to be constrained #1373

Closed
wants to merge 19 commits into from

Conversation

rozyczko
Copy link
Member

@rozyczko rozyczko commented Aug 15, 2019

This is a fix to Richard's issue from #1376

@rozyczko rozyczko added this to the SasView 5.0.1 milestone Aug 15, 2019
@RichardHeenan
Copy link
Contributor

Built this 5.0 branch locally, previous project saved above did not work when loaded, so pulled in my core, shell, drop data sets to start from scratch. Added some constraints, why are they showing as M3:scale=M2.scale ? (i.e. a mix of use of : and . ), then added one on M3.radius_width = M1.radius_width, this failed to operate properly and the constraint disappeared when I added another one, meanwhile getting error messages as below.

Likely several things going on, if my build is OK ????

17:10:06 - ERROR: SasView threw exception: must be str, not NoneType
Traceback (most recent call last):
File "C:\sasview42\sasview\src\sas\qtgui\Perspectives\Fitting\ConstraintWidget.py", line 821, in initializeFitList
self.updateFitLine(tab)
File "C:\sasview42\sasview\src\sas\qtgui\Perspectives\Fitting\ConstraintWidget.py", line 768, in updateFitLine
label = moniker + ":"+ constraint_name[0] + " = " + constraint_name[1]
TypeError: must be str, not NoneType
17:10:18 - ERROR: SasView threw exception: must be str, not NoneType
Traceback (most recent call last):
File "C:\sasview42\sasview\src\sas\qtgui\Perspectives\Fitting\ConstraintWidget.py", line 821, in initializeFitList
self.updateFitLine(tab)
File "C:\sasview42\sasview\src\sas\qtgui\Perspectives\Fitting\ConstraintWidget.py", line 768, in updateFitLine
label = moniker + ":"+ constraint_name[0] + " = " + constraint_name[1]
TypeError: must be str, not NoneType

@RichardHeenan
Copy link
Contributor

RichardHeenan commented Aug 21, 2019

Started afresh, trying to add even simple constraints is not working for me in this branch, and getting more of the error messages. Have you a suitable windows build that I could install instead? (Though I may not be back in until Weds 28th.)

@wpotrzebowski
Copy link
Contributor

5.0 ready for testing on Win

@wpotrzebowski
Copy link
Contributor

@RichardHeenan
Copy link
Contributor

RichardHeenan commented Sep 5, 2019

Installed the windows version for this branch. Not getting very far with testing constraints yet, as just doing a simple fit on one data set, as soon as I try to fit a polydispersity, the fit to radius and radius_pd go completely mad (e.g. radius drops from 30 to 0.3 Ang and pd goes very large, fit gets much worse). The fit results for _pd are also ignoring the max & min values. e.g. try fitting the microemulsion drop or shell contrast to polydisp sphere or core-shell sphere respectively.

With three data sets (core,s hell & drop microemulsions) loaded and models set up, adding constraints did not work properly. I tried to do M1.radius = M3.radius, clicked "add" but M1.scale = M3.radius appeared in the table, after which only one data set appeared at the top instead of three, and I started to get error messages ...
12:21:24 - ERROR: SasView threw exception: must be str, not NoneType
12:22:00 - ERROR: SasView threw exception: list index out of range
12:22:11 - ERROR: SasView threw exception: must be str, not NoneType
12:22:14 - ERROR: SasView threw exception: must be str, not NoneType
12:22:16 - ERROR: SasView threw exception: must be str, not NoneType

@RichardHeenan
Copy link
Contributor

Something is broken in "polydispersity", this likely needs a new ticket.
I compared my local build of sasview ESS_GUI + sasmodels master with release 4.2
Report1 below uses the best fit parameter from 4.2 release, fitting scale, radius & radius_width, with just a compute in my local build, this looks good, BUT hitting fit gives results in report2, which are plain crazy, radius drops from 22 to 0.2 ang, radius_width goes to zero, which is outside the min value set at 0.02. Would some other people please test likewise.

fit_report1_5Sept2019.pdf
fit_report2_5Sept2019.pdf

@rozyczko rozyczko modified the milestones: SasView 5.0.1, SasView 5.1.0 Sep 16, 2019
@rozyczko rozyczko changed the title Allow polydisperse parameters to be constrained WIP. DONT MERGE. Allow polydisperse parameters to be constrained Mar 19, 2020
@rozyczko rozyczko removed the request for review from butlerpd March 19, 2020 13:32
@pkienzle pkienzle marked this pull request as draft August 18, 2020 13:46
@wpotrzebowski wpotrzebowski changed the base branch from ESS_GUI to main October 26, 2020 07:18
@smk78 smk78 added the For Feature Parity Issues to give 5x the same functionality as 4x label Feb 2, 2021
rozyczko and others added 5 commits February 4, 2021 10:54
Created a dictionary within the FittingWidget class, named model_dict,  to contain the main models of SasView. The keys are “standard”, “poly” and “magnet” relating to the main model, polydisperse model and magnet model respectively.
Many functions in the code have been refactored to accept an argument, called model_key, which corresponds to a key in model_dict. This allows the model to be passed to a function without having to pass the bumps object, which can lead to the code misbehaving.
…training or removing a constraint on right click.
Required updating the getSymbolDict function in line with previous commits on this branch
Small edit to the getActiveConstraintList function
@lucas-wilkins
Copy link
Contributor

What's the status of this, is it being actively worked on?

@butlerpd butlerpd added the Discuss At The Call Issues to be discussed at the fortnightly call label May 23, 2022
@butlerpd
Copy link
Member

This needs a bit (a lot?) more love so closing the PR but keeping the branch for more work

@butlerpd butlerpd closed this May 24, 2022
@butlerpd butlerpd removed the Discuss At The Call Issues to be discussed at the fortnightly call label May 24, 2022
@lucas-wilkins
Copy link
Contributor

Reopening, as I'm working on it, and this pages doesn't seem to update without being open

@lucas-wilkins lucas-wilkins reopened this Jun 24, 2022
@lucas-wilkins
Copy link
Contributor

What exactly is remaining here? Happy to chat about it @rozyczko (though I'll be away after today for 3 weeks)

@wpotrzebowski wpotrzebowski added Discuss At The Call Issues to be discussed at the fortnightly call and removed Discuss At The Call Issues to be discussed at the fortnightly call labels Jul 5, 2022
@lucas-wilkins
Copy link
Contributor

Going to close this again for now - its just hanging around.

@rozyczko
Copy link
Member Author

good. I think it was originally created to enable creation of artifacts on Jenkins, where we had them only on PR commits

@butlerpd
Copy link
Member

When you say 3 weeks @lucas-wilkins, I assume that the 2nd week is because you'll be at code camp ... with @rozyczko and the rest of us? Maybe you can sort out what needs doing then?

@lucas-wilkins
Copy link
Contributor

When you say 3 weeks @lucas-wilkins, I assume that the 2nd week is because you'll be at code camp ... with @rozyczko and the rest of us? Maybe you can sort out what needs doing then?

When I said 3 weeks it was 4 months ago!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Feature Parity Issues to give 5x the same functionality as 4x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants