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

Error Loading custom models if name attribute differs from the model file name #1443

Open
butlerpd opened this issue Jan 16, 2020 · 15 comments
Labels
Defect Bug or undesirable behaviour Major Big change in the code or important change in behaviour Plotting Concerns plotting functionality
Milestone

Comments

@butlerpd
Copy link
Member

A user from Lund University has found two bug in the 5.X custom models package (4.2.2 works correctly).

  1. New models that are dropped into the plugin folder while SasView is running do not get picked up on the fly. This is true of 4.x as well which is why the option to "load plugin" under " Plugin Model Operations" was added. This option seems to have been lost in the transition to 5.x and clearly needs to be restored.

  2. 5.x incorrectly assumes the python file name is the same as the UI name given within the model file via the "name" attribute, appended with a .py. This of course is not the case. If it were, the name property would be pointless.

@butlerpd butlerpd added Defect Bug or undesirable behaviour SasView Bug Fixing Blocker Prevents a different issue from being resolved labels Jan 16, 2020
@butlerpd butlerpd added this to the SasView 5.0.1 milestone Jan 16, 2020
@rozyczko
Copy link
Member

rozyczko commented Jan 16, 2020

  1. The automatic loading of plugins happens on any GUI invoked change in the plugins. Indeed, manually placing a plugin in the directory will not make it discoverable until the application restart. Adding the explicit 'load plugins' action might help but since the workaround is obvious I don't really agree with the "blocker" status.
  2. Can't reproduce this - if I create a plugin with name test.py and later modify its "name" attribute inside the file to say "test 2", this modified plugin loads in just fine in 5.0, displaying correctly "test 2" in the list of models. Is this what you say is working incorrectly? If so, please let me know the steps required to reproduce the problem.

@butlerpd
Copy link
Member Author

Sorry ... I was being lazy. For number 1 I would have put critical (still higher than major because we added it early on to 4.x because of confusion and figure it will only be worse now that people are used to it? but maybe not), but indeed not blocker.

The blocker was because of point 2 which I am very surprised didn't reproduce. Actually we did reproduce on 2 different windows 10 machines. Further it was failing in 5.0 release and on 5.0.1 nightly jenkins build #317 as well as probably 5.0.1 release candidate 1 (though could have been a later version). I eventually realized the reason the student could not load the plugin by noticing that an error was being generated "cannot find file xxxx" where xxx was the name attribute value not the file name. When we renamed the files to match the name attribute it worked on both computers.

I will try to investigate a bit more, starting by seeing what happens for me when creating a test.py

@pkienzle
Copy link
Contributor

Perhaps a dialog to add models to the plugin directory?

That way we don't have to document ~/.sasview/plugins as the path, which can be difficult to navigate to on Mac since dot files are hidden from the finder, and the GUI will know to reset the plugin list.

@rozyczko
Copy link
Member

Added plugin file loader to the Plugin Manager, under "Add file...".

@smk78
Copy link
Contributor

smk78 commented Jan 24, 2020

Testing 5.0.1 Build 321 (left image) and locally-built from ESS_GUI branch (right image) I can't see the plugin file loader.

issue1443

@rozyczko
Copy link
Member

It's because it lives in a separate branch. Sorry - should've added this to the description.
Modified the code a bit after PK's comment - the Copy action now copies also corresponding C file, if present.
Merged into ESS_GUI, so this is available in the main branch as well.

@smk78
Copy link
Contributor

smk78 commented Feb 10, 2020

If this was merged into ESS_GUI 10 days ago shouldn't the plugin file loader be available in the 5.0.1 release? I also just tried pulling and building the ESS_GUI branch and it's not appearing there either.

@rozyczko
Copy link
Member

I guess there's some confusion here - the option resides in the Plugin Manager, as seen in the attached screenshot of 5.0.1.

Untitled

@smk78
Copy link
Contributor

smk78 commented Feb 10, 2020

Ahh!

So with the 5.0.1 release running, I went into my plugin folder and made a copy of a plugin that was available to use in a 5.0.1 FitPage. I renamed the .py file to something new, and edited the model name inside to something new also. I then went to the plugin manager > Add file... and selected the new .py file. I get this:

image

which seemed odd?

I then clicked Yes and the Log Explorer reported

17:27:43 - INFO: "C:\Program Files\SasView-5.0.1\tinycc-data\tcc.exe" -shared -rdynamic -Wall C:\Users\smk78\AppData\Local\Temp\sas64_sphere_bmrnfy7v.c -o C:\Users\smk78\.sasmodels\compiled_models\sas64_sphere.so
17:27:44 - INFO: "C:\Program Files\SasView-5.0.1\tinycc-data\tcc.exe" -shared -rdynamic -Wall C:\Users\smk78\AppData\Local\Temp\sas64_hardsphere_p0w7v4cw.c -o C:\Users\smk78\.sasmodels\compiled_models\sas64_hardsphere.so
17:27:45 - INFO: Note: fred8 has no user defined tests.
test_sphere@hardsphere_dll (sasmodels.model_test._hide_model_case_from_nose.<locals>.ModelTestCase) ... ERROR
Traceback (most recent call last):
  File "C:\Program Files\SasView-5.0.1\sasmodels\model_test.py", line 236, in run_all
    results = [self.run_one(model, test) for test in P_tests]
  File "C:\Program Files\SasView-5.0.1\sasmodels\model_test.py", line 236, in <listcomp>
    results = [self.run_one(model, test) for test in P_tests]
  File "C:\Program Files\SasView-5.0.1\sasmodels\model_test.py", line 336, in run_one
    F, Fsq, R_eff, volume, volume_ratio = call_Fq(kernel, pars)
  File "C:\Program Files\SasView-5.0.1\sasmodels\direct_model.py", line 82, in call_Fq
    return calculator.Fq(call_details, values, cutoff, is_magnetic, R_eff_type)
  File "C:\Program Files\SasView-5.0.1\sasmodels\kernel.py", line 155, in Fq
    radius_effective_mode)
  File "C:\Program Files\SasView-5.0.1\sasmodels\kernel.py", line 189, in _call_kernel
    raise NotImplementedError()
NotImplementedError: fred8-dll
17:30:55 - ERROR: Traceback (most recent call last):
  File "site-packages\sasview-5.0.1-py3.6-win-amd64.egg\sas\qtgui\Utilities\PluginManager.py", line 159, in onAddFile
  File "shutil.py", line 241, in copy
  File "shutil.py", line 104, in copyfile
shutil.SameFileError: 'C:/Users/smk78/.sasview/plugin_models/fred8.py' and 'C:\\Users\\smk78\\.sasview\\plugin_models\\fred8.py' are the same file

and if I look in the list of plugins in the FitPage the model is not there.

@smk78
Copy link
Contributor

smk78 commented Feb 10, 2020

Let me also quickly add that whilst that example was a P(Q)*S(Q) plugin, the same behaviour is demonstrated by P(Q)+P(Q) plugins.

@rozyczko
Copy link
Member

Yes, I can reproduce this issue - I should've added a catch for shutil.copy for copying files to itself.
Will fix in 5.0.2.
Please note that once the plugin is in the right location (.sasview/plugin_models) there is no need to add it explicitly with Add File. It will be picked up automatically.
Add File is for users to add their plugins to the plugin directory without them having to know/remember the plugin directory location.

@rozyczko rozyczko modified the milestones: SasView 5.0.1, SasView 5.0.2 Feb 11, 2020
@wpotrzebowski wpotrzebowski added the Plotting Concerns plotting functionality label Mar 17, 2020
@butlerpd butlerpd added Critical High priority and removed Blocker Prevents a different issue from being resolved labels Mar 31, 2020
@butlerpd butlerpd modified the milestones: SasView 5.0.2, SasView 5.0.3 Mar 31, 2020
@butlerpd
Copy link
Member Author

As per discussion today moving this to 5.0.3 milestone. Also downgrading from blocker to critical since:

  1. It obviously is not a blocker if we are moving it
  2. There are ways to workaround the issue
  3. This is a much bigger project and will need some time to sort and should not prevent other improvements from being delivered as they become available.

@butlerpd butlerpd added Major Big change in the code or important change in behaviour and removed Critical High priority labels May 25, 2020
@butlerpd
Copy link
Member Author

butlerpd commented May 25, 2020

Having retested the original problem reported using 5.0.2 release I can in fact still reproduce both issues but will agree that issue 1 actually minor. Here is an update from more careful testing:

  1. The check for any changes to models in the plugin folder only happens on entry into fitting (e.g. from invariant to fitting) or from changes using Manage Custom Models if one dumps a file into the plugin folder manually (not using add file) while fitting perspective is active (or likewise deletes it manually) the fitting perspective is unaware and the available models options will not change. Given that "add file" from the manger does this and that switching to another perspective and back will also fix it I don't think this needs fixing any more
  2. If I take a working plugin model (MyModelName.py) and then edit it and only change the name attribute (i.e. the line that says name = "MyNewModelName" immediately after the import statements, it still shows up in the list of plugins but with the name MyNewModelName not MyModelName. Choosing it will throw an error because SasView is now looking for a non existant sasmodels.models.MyModelName (i.e. looking for a def function with the python file name so apparently sasmodels is setting the function name to that listed in "name"). I append a traceback in the next comment. That said this is probably not a critical issue given all the other critical issues we have.
  3. the error reported by @smk78 is yet another problem (SasView trying to write ontop of itself?)

Bottom line is that I suggest we should close this issue and create two new ones for point 2 and 3 but both would be major not critical. For now I will just call this issue as major
I think

@butlerpd
Copy link
Member Author

Here is the Traceback which appears in the console log and sasview.log (but strangely not the plugin.log). The python file name here is TestSinFunc.py and the name attribute line is given as:
name = "TestFunc"

2020-05-25 11:16:50,201 : ERROR : sas.qtgui.Perspectives.Fitting.FittingWidget (FittingWidget.py:2345) :: Can't find the model No module named 'sasmodels.models.TestSinFunc'
2020-05-25 11:16:50,202 : ERROR : sas.qtgui.Perspectives.Fitting.FittingWidget (FittingWidget.py:174) :: Traceback (most recent call last):
  File "site-packages\sasview-5.0.2-py3.6.egg\sas\qtgui\Perspectives\Fitting\FittingWidget.py", line 1170, in onSelectModel
  File "site-packages\sasview-5.0.2-py3.6.egg\sas\qtgui\Perspectives\Fitting\FittingWidget.py", line 1307, in respondToModelStructure
  File "site-packages\sasview-5.0.2-py3.6.egg\sas\qtgui\Perspectives\Fitting\FittingWidget.py", line 2293, in SASModelToQModel
  File "site-packages\sasview-5.0.2-py3.6.egg\sas\qtgui\Perspectives\Fitting\FittingWidget.py", line 3317, in addExtraShells
  File "site-packages\sasview-5.0.2-py3.6.egg\sas\qtgui\Perspectives\Fitting\FittingUtilities.py", line 55, in getMultiplicity
  File "site-packages\sasview-5.0.2-py3.6.egg\sas\qtgui\Perspectives\Fitting\FittingUtilities.py", line 48, in getIterParams
AttributeError: 'NoneType' object has no attribute 'iq_parameters'

@butlerpd butlerpd modified the milestones: SasView 5.0.3, SasView 5.0.5 May 2, 2021
@butlerpd
Copy link
Member Author

butlerpd commented Feb 1, 2022

Testing 5.0.5RC2 (windows 10) the original points remain true. In particular for point 2, I created a python model called polynomial.py using the sasview new model editor. Actually it is a simple quadratic with the three coefficients as parameters. It works fine for a number of tests. For this test I used the built in model editor to change the name attribute to poly_test2. Now choosing a different category then back to plugins, the new name appears in the dropdown: poly_test2. Selecting it however produces the single line error in the log: `Can't find the model No module nameds 'sasmodels.model.polynomial'

Exiting the application then restarting it, then selecting Plugin Models again and poly_test2 the traceback shown in the previous post of May 2020 is thrown.

@butlerpd butlerpd modified the milestones: SasView 5.0.5, SasView 5.1.0 Feb 1, 2022
@butlerpd butlerpd changed the title Error with custom models in 5.0 Error Loading custom models if name attribute differs from the model file name Feb 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Defect Bug or undesirable behaviour Major Big change in the code or important change in behaviour Plotting Concerns plotting functionality
Projects
None yet
Development

No branches or pull requests

6 participants