BhallaLab / moose-core

C++ basecode and python scripting interface
https://moose.ncbs.res.in
GNU General Public License v3.0
15 stars 27 forks source link

Attribute values do not persist on MS Windows #483

Closed subhacom closed 2 months ago

subhacom commented 2 months ago

On Windows it seems that when an object is created and its attributes are assigned values inside a function, the attribute values do not persist after returning from the function. Minimal working example:

import moose

Ek = -90e-3
Gbar = 1e-6
Xpower = 3
Ypower = 1

def create_channel(soma):
    chan = moose.HHChannel(f'{soma.path}/chan')
    chan.Ek = Ek
    chan.Gbar = Gbar
    chan.Xpower = Xpower
    chan.Ypower = Ypower
    return chan

soma = moose.Compartment('soma')
chan = create_channel(soma)
assert chan.Ek == Ek, f'Ek mismatch: set {Ek}, got {chan.Ek}'
assert chan.Gbar == Gbar, f'Gbar mismatch: set {Gbar}, got {chan.Gbar}'
assert chan.Xpower == Xpower, f'Xpower mismatch: set {Xpower}, got {chan.Xpower}'
assert chan.Ypower == Ypower, f'Ypower mismatch: set {Ypower}, got {chan.Ypower}'
print('Attributes lasted outside function call')

Output:

Traceback (most recent call last):
  File "C:\Users\raysu\Documents\src\moose-core\tests\core\test_memory_sanity.py", line 37, in <module>
    assert chan.Ek == Ek, f'Ek mismatch: set {Ek}, got {chan.Ek}'
           ^^^^^^^^^^^^^
AssertionError: Ek mismatch: set -0.09, got 1.35675e-319
subhacom commented 2 months ago

With a few print statements, it looks like an issue with multiple inheritance and virtual function. Putting print statements:

The debug prints are inserted as follows:

void ChanBase::setEk( const Eref& e, double Ek )
{
  cout << "ChanBase::setEk Ek set:" << Ek << endl;
    vSetEk( e, Ek );
  cout << "Ek retrieved: " << vGetEk( e ) << endl;
}

And the output shows some possibly garbage value:

>>> import moose
>>> c = moose.HHChannel('c')
>>> c.Ek
ChanBase::getEk 3.39519e-313
2.121995791e-314
>>> c.Ek = 2
ChanBase::setEk Ek set:2
Ek retrieved: 2.97079e-313
>>>

In contrast, setting Xpower works fine. vSetXpower is implemented in HHChannel.cpp and is called in its parent class's HHChannelBase::setXpower.

Edit: The inheritance seems to be a classic diamond shape:

class ChanCommon: public virtual ChanBase
class HHChannelBase: public virtual ChanBase
class HHChannel : public HHChannelBase, public ChanCommon

      ChanBase [ChanBase::setEk calls virtual ChanBase::vSetEk]
       //           \\
      //             \\
ChanCommon            HHChannelBase
ChanCommon::vSetEk         
      \                /
       \              /
           HHChannel
 [HHChannel::vSetEk calls ChanCommon::vSetEk]
subhacom commented 2 months ago

With some debug prints, the Ubuntu/gcc build takes a different run path from the Windows/MS BuildTools, which is quite baffling. I checked with a minimal example that MSBuild Tools does work fine for the diamond-shaped inheritance like above.

Ubuntu/gcc:

> python -c "import moose; c = moose.HHChannel('c'); c.Ek = 3; print(c.Ek)"
ChanBase::setEk: current value Ek=ChanCommon::vGetEk: Ek = 0
0, setting to:3
ChanCommon::vSetEk: Setting Ek to 3
ChanCommon::vSetEk: Ek = 3
**** Ek retrieved: ChanCommon::vGetEk: Ek = 3
3
ChanBase::getEk ChanCommon::vGetEk: Ek = 3
3
ChanCommon::vGetEk: Ek = 3
3.0

Windows/MS BuildTools

> python -c "import moose; c = moose.HHChannel('c'); c.Ek = 3; print(c.Ek)"
ChanBase::setEk: current value Ek=7.21479e-313, setting to:3
**** Ek retrieved: 4.03179e-313
ChanBase::getEk 3.39519e-313
2.121995791e-314

Thus the print statements inside ChanCommon::vSetEk() and ChanCommon::vGetEk() are never carried out in the Windows version, although they work as expected in the Ubuntu build!

subhacom commented 2 months ago

Update: partially fixed by making ChanBase::~ChanBase() virtual. This fixed Ek setting, but not Gbar. Moreover, the debug prints in the ChanCommon::vSetEk(...) are still not executed.

subhacom commented 2 months ago

It seems that the virtual function table is messed up on Windows. The call to vSetGbar() in ChanBase::setGbar() actually calls HHChannel::vSetYpower().

One more issue found and fixed was data member modulation_ was defined in both HHChannelBase and ChanCommon.

Needs closer inspection.

subhacom commented 2 months ago

Fixed in #485