Opened 3 years ago

Closed 3 years ago

#795 closed defect (fixed)

Projects saved in (Mac) 3.1.2 do not open in 4.0

Reported by: smk78 Owned by: krzywon
Priority: blocker Milestone: SasView 4.1.0
Component: SasView Keywords:
Cc: Work Package: SasView Bug Fixing

Description

Reported by hmarti18@…:

Projects saved in Mac version 3.1.2 would not open in 4.0.

User reverted to 3.1.2 and all was well.

This may be fixed in the work Jeff did at the end of Code Camp V towards 4.1 but should be checked.

Change History (24)

comment:1 Changed 3 years ago by pkienzle

This will require quite a bit of work.

In addition to finding the equivalent new model you must also update the parameters to the new names, and in some case change the values. For example (radius, inner_radius) goes to (radius=inner_radius, thickness=outer_radius-radius). We've already got code to go the other direction which we used to test against sasview 3.x (see sasmodels/convert.py); this needs to be turned into forward conversion code, and used to update the parameters in perspectives.fitting.pagestate.PageState._parse_state() after from_xml().

It shouldn't be _that_ hard to do given that we already know all the conversions (except for raspberry), but it'll take some time. Testing could be especially tedious if you do it by hand. Every model needs to be saved and reloaded to make sure the parameters are converted correctly. Given the number of models involved, writing a script to do this would be worthwhile.

The reparameterized models with polydispersity may not be translatable. For example, an outer radius of 100 A with 10% pd will use values in (70, 130). If the inner radius is 90 A, then setting thickness to 10 A with 100% pd, giving values in (0, 40) gives the same result. However, if both inner and outer radius are polydisperse at 10% there is no value of thickness pd which will yield the same set of models, with inner in (63, 117) and outer in (70, 130). If you allow thickness of 67 A to achieve the maximum outer radius when the inner radius is smallest, then the new maximum outer radius will be 184.

Also need to check different polydispersity distributions, 2-D models, and magnetic models are properly saved and restored. Need to make sure parameter uncertainties are reloaded as well, even for parameters that have changed names. Don't need to check constrained fits since they weren't able to be reloaded before.

comment:2 Changed 3 years ago by butler

Can we get the users file to attache here and use as a test case?

Jeff did do substantial work on the save project for 4.1 (in fact the whole code camp) and did do things that he thought handled a lot but not all of the old models - probably for some of the reasons Paul says. Not sure how backwardly compatilbe we can be without getting ourselves in the hole we are in of overly complicated stuff that gets very hard to maitain long term, but we don't want to have users constantly losing stuff every time we make a new release either — a bit of a balancing act here I guess but if we have the real world example we can see if it works and ask Jeff what the state of play is and what his plans/thoughts were vis a vis Paul Kienzle's discussion above?

comment:3 Changed 3 years ago by pkienzle

Sum/product models will require an extra step. Have to recognize that it is a composition model rather than an old style python custom model, identify the component models, and split the p1_* parameters from the p2_* parameters. For each p1_/p2_ set, strip the prefix, rename according to the sasview→sasmodels conversion, then add back the prefix. This may be recursive if it is the sum of sum models for example.

Similarly, S(Q)*P(Q) models will require separation into P(Q) and S(Q) parameters, renaming them separately, then combining them again. It will be a little different since the parts aren't tagged with p1_ and p2_, but in this case you can look up the S(Q) parameters from the S(Q) conversion table.

comment:4 Changed 3 years ago by ajj

I think the better approach would be to not do this "transparently" - we should have a conversion routine that users can run their saved projects from 3.1.2 and 4.0 through to convert them to work in 4.1

We have the added wrinkles that people saving 4.0 files also won't be compatible with 4.1 because some model parameter names have changed since 4.0 was released and also 4.1 will have different style composition models.

comment:5 Changed 3 years ago by pkienzle

sasmodels.convert.convert_model(name, pars) on the ticket776 branch takes an old name and old parameters and returns the new name and new parameters. If called with a new name it does nothing.

Note that this does not convert fit ranges or uncertainty in the fitted value, both of which need to be scaled by 1e6 for SLDs. There are also models which scale by 1e15 and 1e-13.

Also, the multiplicity is not stored as a parameter, but instead stored as multi factor in the page state. This is okay except for spherical_sld, which has been updated so that core is just another layer, and multi factor needs to be incremented. Before calling convert pars['CONTROL'] should be set to multi factor, and on return multi factor should be set to pars.pop('CONTROL').

comment:6 Changed 3 years ago by pkienzle

  • Owner set to krzywon
  • Status changed from new to assigned

comment:7 Changed 3 years ago by krzywon

In 3d8c49af662017ceb292ac3e458fbc07226af1d1/sasview:

#795: Start of the conversion of old save states to new.

comment:8 Changed 3 years ago by krzywon

In 88faface9579f8bf318bf094f003a72c83810b7d/sasmodels:

#795: convert.py converts old style models to new style while keeping legacy funcitons in tact.

comment:9 Changed 3 years ago by krzywon

In 8d067799ab46b98870d397d613f4b18d6962f213/sasmodels:

#795: Properly check for parameters during conversion.

comment:10 Changed 3 years ago by krzywon

In 64c7f3927d89c577cd30adb43291da24b774f6b1/sasview:

#795: Forward conversion of v3.0+ save states working for simple models, but requires testing of complex and reparameratized models.

comment:11 Changed 3 years ago by krzywon

In b4fc0f9db6fcc7cc64b7dc01d3ce4489cef3ff18/sasview:

#795: Orientation parameters returned by sasmodels are now handled properly.

comment:12 Changed 3 years ago by krzywon

In 47fafe9b53dbcd9e41c443aa9d5f55f166d5e8fb/sasmodels:

#795: Old style structure factors are now loaded in properly.

comment:13 Changed 3 years ago by krzywon

In 377c19a242d6cb42a774094d07aef3b20e18f65b/sasview:

#795: Form factor and structure factor parameters are handled separately to avoid conflicts in sasmodels convert.py.

comment:14 Changed 3 years ago by krzywon

In 8898558b3c16f2405cd5f0670926427669d567bc/sasview:

#795: Added a check to see if model names are from sasmodels in SasView? v4+ or from v3.x.y. Conversion tool is skipped if from v4 or later.

comment:15 Changed 3 years ago by krzywon

In f8f5a3788278e81081ae2fe4b57b4cf0ed5b93d1/sasmodels:

#795: Working on reparameterized models: Teubner-Strey and Hollow Cylinder load properly.

comment:16 Changed 3 years ago by krzywon

In db5294eb167ea87b105d85329af888f2b1a980fd/sasview:

#795: Small fix to invariant to check for None type when loading values. Working toward loading rpa and coreshellellipsoid integration.

comment:17 Changed 3 years ago by krzywon

In d2fb4afd60478f3face515abac31b1a3a4763584/sasmodels:

#795: No longer an issue with multiple backgrounds and scales. A handful of model names were slightly off from the save state names. Currently going through model-by-model to finish this.

comment:18 Changed 3 years ago by krzywon

In 077666e2f0fbcdd3d6f6f436258856ac0136e23a/sasmodels:

#795: Gel fit, Lamellar Caille, Debye, and Spherical SLD issues resolved.

comment:19 Changed 3 years ago by krzywon

In 406644adb34ecc0d5b6c6ae338f01a51cf2a20c2/sasview:

#795: A few more models have been properly converted and tested to work. Still to do: 4 models not loading, custom models, and fix an error thrown when negative values are loaded.

comment:20 Changed 3 years ago by krzywon

As of right now, the ticket-795 branches are loading project and analysis files saved in sasview v3.0.0, v3.1.0, v3.1.2, v4.0.1 and the repository. Only two models (RPA and Spherical SLD) are not loading 100% correctly. For the spherical SLD model, the fit function that can be selected in 3.1.2 is not loading properly in the repo, but all other parameters are okay. For the RPA model, the selection specific parameters (based on which type of block coploymer is selected) are not loaded properly.

There is also a versioning system built in to future-proof the system. Any time a model name is changed, or model parameters are changed, sasmodels/conversion_table.py should be modified to ensure forward compatibility of all save states.

Usage:

<Old_SasView_Version> :
{
    "<new_model_name>" : [
        "<old_model_name>",
        {
            "<new_parameter_name>" : "<old_parameter_name>"
        }
}

If the model name has not changed, but the parameters have, <old_model_name> and <new_model_name> will be the same. If no parameter names have changed, just the model name, the parameter dictionary can be left empty.

comment:21 Changed 3 years ago by krzywon

In 063304830c63d7d76004b4507d5846bbb5de0be5/sasview:

#795: String parameters are now handled properly. Try/except blocks that gave save state compatibility from SasView? v2.2.1 to v3.0.0 were removed.

comment:22 Changed 3 years ago by krzywon

In 16fd9e577e93225ba97f055c5b9ea75b4904d7a6/sasmodels:

#795: A check for the spherical SLD is now in place and versioning is now based off of the SasView? version being converted from.

comment:23 Changed 3 years ago by krzywon

The original pull request (https://github.com/SasView/sasview/pull/22) has been closed now that this and work done for #827 have been merged into one. The new pull request (https://github.com/SasView/sasview/pull/30) contains all work done for both tickets.

comment:24 Changed 3 years ago by GitHub <noreply@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 2510b9b9dd6b1865d897555b1fd71086a65f89d7/sasview:

Merge pull request #30 from SasView?/ticket-827

Fixes #795, #819, #827, #847

Note: See TracTickets for help on using tickets.