ForceFcn for pockets C1&2 are not well defined

[sebdenis]

Hi,

there is an issue regarding the definition of ForceFcnmethods for pockets C1 and C2 done in pre_run method in scroll.core. The code concerned is

        for i in range(10):
            try:
                Fcn = lambda theta, geo: symm_scroll_geo.C1_forces(theta, i+1, geo)
                self.CVs['c1.' + str(i+1)].ForceFcn = Fcn
            except:
                pass
            try:
                Fcn = lambda theta, geo : symm_scroll_geo.C2_forces(theta, i+1, geo)
                self.CVs['c2.' + str(i+1)].ForceFcn = Fcn
            except:
                pass

Doing this way, ForceFcndoes not 'freeze' the value of parameter i. In the end, all the ForceFcn functions are equal to lambda theta, geo: symm_scroll_geo.C1_forces(theta, 10, geo). See https://stackoverflow.com/questions/3431676/creating-functions-in-a-loop for more explanation. To fix it, one solution is to use partial from module functools. See https://stackoverflow.com/questions/55396224/how-to-create-a-list-of-functions-using-one-function-with-different-arguments.

[ibell]

Thanks for the bug report. This is a pretty serious one, I have no idea how it survived before. My commit (https://github.com/ibell/pdsim/commit/cba2a5f738858208c94eda1c25926d8ba920f108) should have fixed it I think

[ibell]

Please do let me know if this fixes the problem for you

[sebdenis]

It's ok for me. Bug fixed!

[ibell]

Great!

On Tue, Apr 16, 2019, 2:16 AM S DENIS notifications@github.com wrote:

It's ok for me. Bug fixed!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ibell/pdsim/issues/31#issuecomment-483559357, or mute the thread https://github.com/notifications/unsubscribe-auth/ABxha4_WFCV2jWwGQ4CZjonP-hu07DEjks5vhYbggaJpZM4crk44 .

[sebdenis]

Actually, I have an issue. With a fresh install of PDSim, I retried it and got the followed error :

no forces for c1.1 with error: C2_forces() takes at least 3 positional arguments (2 given) and something similar with c1.2, c2.1, c2.2. I think the problem is that you want to fix the parameter alpha, which is not the last one of functions symm_scroll_geo.C1_forces and symm_scroll_geo.C2_forces. I solved the problem using the code below:

  Fcn = lambda theta, geo, alpha: symm_scroll_geo.C1_forces(theta, alpha, geo)

  self.CVs['c1.' + str(i+1)].ForceFcn = functools.partial(Fcn, alpha=i+1)

and the same for c2.

[ibell]

I don't think that fix works in general, or at least not in Python 3:

import functools

def f(a1,a2,a3):
    print(a1,a2,a3)    

ff = functools.partial(f, a2=3)
ff(4,5)

yields

TypeError                                 Traceback (most recent call last)
<ipython-input-1-2c64f0f6e828> in <module>
      4 import functools
      5 ff = functools.partial(f, a2=3)
----> 6 ff(4,5)

TypeError: f() got multiple values for argument 'a2'

[ibell]

But this works:

import functools

def f(a1,a2,a3):
    print(a1,a2,a3)

def ff(a1,a3,a2):
    f(a1,a2,a3)

fff = functools.partial(ff, a2=3)
fff(4,5)

[ibell]

I think this is fixed now

[sebdenis]

Tested and approved in Python 2. Problem solved for me.