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

updated custom models not being recompiled (Trac #576) #708

Closed
pkienzle opened this issue Mar 30, 2019 · 16 comments
Closed

updated custom models not being recompiled (Trac #576) #708

pkienzle opened this issue Mar 30, 2019 · 16 comments
Assignees
Labels
Defect Bug or undesirable behaviour Major Big change in the code or important change in behaviour
Milestone

Comments

@pkienzle
Copy link
Contributor

pkienzle commented Mar 30, 2019

When updating the custom model within sasview on opencl the model doesn't always get updated.

When compiling to a dll, compile fails because dll is already loaded.

Migrated from http://trac.sasview.org/ticket/576

{
    "status": "closed",
    "changetime": "2016-10-09T15:21:40",
    "_ts": "2016-10-09 15:21:40.699626+00:00",
    "description": "When updating the custom model within sasview on opencl the model doesn't always get updated.\n\nWhen compiling to a dll, compile fails because dll is already loaded.",
    "reporter": "pkienzle",
    "cc": "",
    "resolution": "fixed",
    "workpackage": "SasView Bug Fixing",
    "time": "2016-05-21T02:35:23",
    "component": "SasView",
    "summary": "updated custom models not being recompiled",
    "priority": "major",
    "keywords": "",
    "milestone": "SasView 4.1.0",
    "owner": "wojciech",
    "type": "defect"
}
@pkienzle pkienzle added this to the SasView 4.1.0 milestone Mar 30, 2019
@pkienzle pkienzle added Defect Bug or undesirable behaviour Incomplete Migration Major Big change in the code or important change in behaviour and removed Incomplete Migration labels Mar 30, 2019
@wpotrzebowski
Copy link
Contributor

Trac update at 2016/08/09 09:15:58:

  • wojciech changed owner from "" to "wojciech"
  • wojciech changed status from "new" to "assigned"

@pkienzle
Copy link
Contributor Author

Trac update at 2016/08/16 15:13:48: pkienzle commented:

Can force use of DLL rather than OpenCL using the following in the python shell:

    import sasmodels.core
    sasmodels.core.USE_OPENCL = False

@pkienzle
Copy link
Contributor Author

Trac update at 2016/08/16 15:46:51: pkienzle commented:

To reproduce, create a custom model using Fitting>Edit Custom Model>New, setting model to e.g.,

return a + cos(b*x)

You can then use this model on the fit page. When the model is updated, e.g., to:

return a + cos(b*x) + 100

the new model is not updated.

The same is true for C models. Update the model to:

form_volume = """return 1.0;"""
Iq = """return a + cos(b*q) + 1;"""
Iqxy = None

Test model update and reload on mac, windows wt/ opencl and windows wt/ tinycc.

@butlerpd
Copy link
Member

Trac update at 2016/08/18 16:13:00: butler changed _comment0 from:

OpenCL fixed, DLL fails.

To get the DLL to work, first remove the frozen check in sasmodels.kerneldll.make_dll, then restructure the DllKernel so that it doesn't hold any references to the DLL.

Follow the instructions to reproduce above, but with sasmodels.core.HAVE_OPENCL=False.

We can do this by wrapping the ct.CDLL in a class with methods for the different kernels. Instead of calling the kernels directly, calls from DllKernel will go through this object. DllModel may be could do this if we can guarantee there is only ever one DllModel instance for a particular dll, for example, by caching DllModel within load_dll(). Then its internal dll can be released, recompiled and reloaded with any attached kernels automatically updated.

to:

1473041132931895

@pkienzle
Copy link
Contributor Author

Trac update at 2016/08/18 16:13:00: pkienzle commented:

OpenCL fixed, DLL fails.

To get the DLL to work, first remove the frozen check in sasmodels.kerneldll.make_dll, then restructure the !DllKernel so that it doesn't hold any references to the DLL.

Follow the instructions to reproduce above, but with sasmodels.core.HAVE_OPENCL=False.

We can do this by wrapping the ct.CDLL in a class with methods for the different kernels. Instead of calling the kernels directly, calls from !DllKernel will go through this object. !DllModel may be could do this if we can guarantee there is only ever one !DllModel instance for a particular dll, for example, by caching !DllModel within load_dll(). Then its internal dll can be released, recompiled and reloaded with any attached kernels automatically updated.

@ajj
Copy link
Member

ajj commented Mar 30, 2019

Trac update at 2016/09/06 10:06:36: ajj commented:

A student I am working with (Windows / no OpenCL) noticed this problem and currently is working round it by deleting the dll to force a rebuild.

He also noticed that python models will rebuild if secondary functions are changed, but not if Iq and Iqxy are changed. Which is odd.

@ajj
Copy link
Member

ajj commented Mar 30, 2019

Trac update at 2016/09/26 08:41:46: ajj commented:

Attached model (hexagonal_cylinder.py) is the one that was causing problems - there was different rebuild behaviour between modifying secondary functions (e.g. Reflections() ) vs modifying Iq or Iqxy.

@ajj
Copy link
Member

ajj commented Mar 30, 2019

Trac update at 2016/09/26 08:42:02: ajj changed attachment from "" to "hexagonal_cylinder.py"

@wpotrzebowski
Copy link
Contributor

Trac update at 2016/09/27 11:27:03: wojciech commented:

I did some testing and it seems that python models behave properly (are being updated). This also applies to hexagonal_cylinder.py (at least according to my test).

However, the problem still persist if the plugin models have c code sections like:
form_volume = """return 1.0;"""
Iq = """return a + cos(b*q) + 1;"""
Iqxy = None

These models don't update but !SasView doesn't crush.

So this bug seems to apply to rather limited number of cases (plugins with C code embedded and no OpenCL). I guess it shouldn't affect too many users. Shouldn't we wait with fixing it until full 4.0 is out?

@ajj
Copy link
Member

ajj commented Mar 30, 2019

Trac update at 2016/09/27 14:40:29: ajj changed _comment0 from "As decided at the fortnightly meeting Sep 28, 2016 we feel this is good enough for release 4.0 and it would be dangerous to try to fix the rest 3 days before the final release -- moving to 4.1" to "1474988079349987"

@butlerpd
Copy link
Member

Trac update at 2016/09/27 14:40:29:

  • butler commented:

As decided at the fortnightly meeting Sep 27, 2016 we feel this is good enough for release 4.0 and it would be dangerous to try to fix the rest 3 days before the final release -- moving to 4.1

  • butler changed milestone from "SasView 4.0.0" to "SasView 4.1.0"

@wpotrzebowski
Copy link
Contributor

Trac update at 2016/10/05 20:59:19: wojciech commented:

In changeset 9acbd37c401f7d905e67370f7361a35762628e75:

#!CommitTicketReference repository="sasmodels" revision="9acbd37c401f7d905e67370f7361a35762628e75"
Unloading lib from ctypes by triggering release function in kerneldll ref #708

@ajj
Copy link
Member

ajj commented Mar 30, 2019

Trac update at 2016/10/06 18:24:33: ajj commented:

In changeset 886fa25dbb18c6bb0e504b4c4231771e0f26d910:

#!CommitTicketReference repository="sasmodels" revision="886fa25dbb18c6bb0e504b4c4231771e0f26d910"
Ticket576 - updated custom models not being recompiled (#239)

* OpenCL drivers installation has been added

* Moved compilation.rst to gpu_computations.rst (more propreiate name)

* Check-ups for 576

* Checks for #708

* Unloading lib from ctypes by triggering release function in kerneldll ref #708

* Import clean-ups in kerneldll

@wpotrzebowski
Copy link
Contributor

Trac update at 2016/10/06 20:20:06: wojciech commented:

In changeset 5b37fd681b84af2a1adc33b956ca441b19da76fe:

#!CommitTicketReference repository="sasmodels" revision="5b37fd681b84af2a1adc33b956ca441b19da76fe"
Needed to try except clause, otherwise was failing when trying disatch model on Windows without precompiled models - ref #708

@wpotrzebowski
Copy link
Contributor

Trac update at 2016/10/08 19:30:01: wojciech commented:

In changeset 47f2b5d:

#!CommitTicketReference repository="sasview" revision="47f2b5d45af5a67c5de39b0539edcbfbe5acbe7c"
Skipping extra Iq evaluation by not calling draw_model function from set polydisperisty parameters routine, ref #756 and #708

@wpotrzebowski
Copy link
Contributor

Trac update at 2016/10/09 15:21:40:

  • wojciech changed resolution from "" to "fixed"
  • wojciech changed status from "assigned" to "closed"

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
Projects
None yet
Development

No branches or pull requests

4 participants