fumitoh / modelx

Use Python like a spreadsheet!
https://modelx.io
GNU Lesser General Public License v3.0
96 stars 21 forks source link

cell with *args not saved properly with the model #23

Closed alebaran closed 4 years ago

alebaran commented 4 years ago

There is a bug, when saving a model with a cell containing *args. The following code

from modelx import *
m, s = new_model(), new_space('s')

@defcells
def a(*args):
    return b()

@defcells
def b():
    return 1

a(1, 2)
m.save('m.mx')
m2 = open_model('m.mx', 'm2')
m2.s.b[()] = 2

Triggers an error

Traceback (most recent call last):
  File "...\Anaconda3\lib\site-packages\IPython\core\interactiveshell.py", line 3326, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-2-7aa69d0eb43e>", line 15, in <module>
    m2.s.b[()] = 2
  File "...\lib\site-packages\modelx\core\cells.py", line 97, in __setitem__
    self._impl.set_value(tuplize_key(self, key), value)
  File "...\lib\site-packages\modelx\core\cells.py", line 611, in set_value
    self._store_value(key, value, True)
  File "...\lib\site-packages\modelx\core\cells.py", line 623, in _store_value
    self.clear_value(*key)
  File "...\lib\site-packages\modelx\core\cells.py", line 644, in clear_value
    self._model.clear_descendants(node)
  File "...\lib\site-packages\modelx\core\model.py", line 220, in clear_descendants
    del node[OBJ].data[node[KEY]]
KeyError: (((1, 2),),)

Note that the same call works for the model without saving: m.s.b[()] = 2

It seems that saving adds an extra tuple layer to the *args: (((1, 2),),)

This issue is blocking for me: if not fixed I'll need to significantly redesign / rebuild the code base.

fumitoh commented 4 years ago

modelx doesn't allow variable length parameters (*args or *kwargs). What's the reason you have to put `` before args?

alebaran commented 4 years ago

*args actually works, except for saving part. I need a layer, which sums up results from all dynamic spaces for each of the variables. And I don't want to program it for each variable separately. Here is how I do it:

from modelx import *
m = new_model()
def s_arg(id):
    pass
s = new_space('s', formula=s_arg)
@defcells
def a(i):
    return i
@defcells
def b(i, j):
    return i * j
def tot_arg(cell):
    pass
tot = new_space('tot', formula=tot_arg)
m.m = m
@defcells
def Sum(*args):
    return sum([m.s(id).cells[cell](*args[0]) for id in range(0,10)])
tot('a').Sum(2)
tot('b').Sum(2, 3)
alebaran commented 4 years ago

In reality I go a bit further to make further referencing cleaner:

from modelx import *
m = new_model()
def s_arg(id):
    pass
s = new_space('s', formula=s_arg)
@defcells
def a(i):
    return i
@defcells
def b(i, j):
    return i * j

def tot_cell_arg(cell):
    pass
tot_cell = new_space('tot_cell', formula=tot_cell_arg)
m.m = m
@defcells
def Sum(*args):
    return sum([m.s(id).cells[cell](*args[0]) for id in range(0,10)])

def tot_arg():
    refs = {cell: m.tot_cell(cell).Sum for cell in m.s.cells}
    return {'refs': refs}

tot = new_space('tot', formula=tot_arg)
tot().a(2)
tot().b(2, 3)
fumitoh commented 4 years ago

How about changing your first example to this

from modelx import *
m = new_model()
def s_arg(id):
    pass
s = new_space('s', formula=s_arg)
@defcells
def a(i):
    return i
@defcells
def b(i, j):
    return i * j
def tot_arg(cell):
    pass
tot = new_space('tot', formula=tot_arg)
m.m = m
@defcells
def Sum(args):   # no *
    return sum([m.s(id).cells[cell](*args) for id in range(0,10)])    # unpack with *

tot('a').Sum((2,))   # pass in tuple
tot('b').Sum((2,3)) # pass in tuple
alebaran commented 4 years ago

In reality I go a bit further to make further referencing cleaner:

from modelx import *
m = new_model()
def s_arg(id):
    pass
s = new_space('s', formula=s_arg)
@defcells
def a(i):
    return i
@defcells
def b(i, j):
    return i * j

def tot_cell_arg(cell):
    pass
tot_cell = new_space('tot_cell', formula=tot_cell_arg)
m.m = m
@defcells
def Sum(*args):
    return sum([m.s(id).cells[cell](*args[0]) for id in range(0,10)])

def tot_arg():
    refs = {cell: m.tot_cell(cell).Sum for cell in m.s.cells}
    return {'refs': refs}

tot = new_space('tot', formula=tot_arg)
tot().a(2)
tot().b(2, 3)

How do you usually approach summing across dynamic spaces?

fumitoh commented 4 years ago

I've never tried it. Definitely need some work to make it more elegant. Did my solution above work?

alebaran commented 4 years ago

How about changing your first example to this

from modelx import *
m = new_model()
def s_arg(id):
    pass
s = new_space('s', formula=s_arg)
@defcells
def a(i):
    return i
@defcells
def b(i, j):
    return i * j
def tot_arg(cell):
    pass
tot = new_space('tot', formula=tot_arg)
m.m = m
@defcells
def Sum(args):   # no *
    return sum([m.s(id).cells[cell](*args) for id in range(0,10)])    # unpack with *

tot('a').Sum((2,))   # pass in tuple
tot('b').Sum((2,3)) # pass in tuple

The issue is that I need a clean syntax, which is fully interchangeable, with the original one. Here is the complete example:

from modelx import *
m = new_model()
def s_base_arg(id):
    pass
s_base = new_space('s_base', formula=s_base_arg)
@defcells
def a(i):
    return i
@defcells
def b(i, j):
    return i * j
def tot_cell_arg(cell):
    pass
tot_cell = new_space('tot_cell', formula=tot_cell_arg)
m.m = m
@defcells
def Sum(*args):
    return sum([m.s_base(id).cells[cell](*args[0]) for id in range(0,10)])

def s_arg(id):
    if id == -1:
        refs = {cell: m.tot_cell(cell).Sum for cell in m.s_base.cells}
    else:
        refs = {cell: m.s_base(id).cells[cell] for cell in m.s_base.cells}
    return {'refs': refs}
s = new_space('s', formula=s_arg)
s(-1).a(2)
s(0).a(2)
s(-1).b(2, 3)
s(0).b(2, 3)

Both s(-1) and s(id) are used in the further code in an interchangeable way. I mean with this that I need to perform the same calculations for both and I don't want to duplicate the code base.

alebaran commented 4 years ago

I found a way:

from modelx import *
m = new_model()
def s_base_arg(id):
    pass
s_base = new_space('s_base', formula=s_base_arg)
@defcells
def a(i):
    return i
@defcells
def b(i, j):
    return i * j

class Tot_func:
    def __init__(self, space, cell):
        self.cell = cell
        self.space = space

    def Sum(self, *args):
        return sum([self.space(id).cells[self.cell](*args) for id in range(0, 10)])

m.Tot_func = Tot_func
m.m = m
def s_arg(id):
    if id == -1:
        refs = {cell: m.Tot_func(m.s_base, cell).Sum for cell in m.s_base.cells}
    else:
        refs = {cell: m.s_base(id).cells[cell] for cell in m.s_base.cells}
    return {'refs': refs}
s = new_space('s', formula=s_arg)
s(-1).a(2)
s(0).a(2)
s(-1).b(2, 3)
s(0).b(2, 3)
fumitoh commented 4 years ago

Good. One minor improvement: no need to put [] within sum