search

SDXorg / pysd

System Dynamics Modeling in Python
http://pysd.readthedocs.org/
MIT License
386 stars 88 forks source link

ValueError is raised when delay_time argument in DelayFixed has dimensions #401

Open rogersamso opened 1 year ago

rogersamso commented 1 year ago

When calculating the order, the following ValueError is raised:

ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

This test model reproduces the error:


delayed_flow[liquids]= DELAY_FIXED (
    flow[liquids], TIME_DELAY[liquids,coord1] , flow[liquids])
    ~   
    ~       |

dimension2:
    coord1, coord2
    ~   
    ~       |

flow[water]=
    100 + STEP(20, 10) ~~|
flow[oil]=
    130 + STEP(20, 10)
    ~   
    ~       |

liquids:
    water, oil
    ~   
    ~       |

TIME_DELAY[oil,coord1]=
    20 ~~|
TIME_DELAY[water,coord1]=
    40 ~~|
TIME_DELAY[oil,coord2]=
    0 ~~|
TIME_DELAY[water,coord2]=
    5
    ~   
    ~       |
enekomartinmartinez commented 1 year ago

The current implementation of any DELAY function allows only a 0-dim positive integer delay_order (in DELAY_FIXED delay_time should be a 0-dim multiple of time_step).

If delay order depends on the dimension, I would suggest splitting your delay equation in equations of the same order. Implementation in Python could be tricky, so it is easier to split the Vensim code, so 1 delay object is created for each order in the translated file.

enekomartinmartinez commented 1 year ago

Another option would be to automatize PySD to create several delay objects following the dimensions of delay_order (or delay_time in DELAY_FIXED), But this would take more time. So it depends on your needs. My recommendation would be writing Vensim code in this way:

delayed_flow[oil]= DELAY_FIXED (
    flow[oil], TIME_DELAY[oil,coord1] , flow[oil])
    ~~ |
delayed_flow[water]= DELAY_FIXED (
    flow[water], TIME_DELAY[water,coord1] , flow[water])
    ~   
    ~       |

dimension2:
    coord1, coord2
    ~   
    ~       |

flow[water]=
    100 + STEP(20, 10) ~~|
flow[oil]=
    130 + STEP(20, 10)
    ~   
    ~       |

liquids:
    water, oil
    ~   
    ~       |

TIME_DELAY[oil,coord1]=
    20 ~~|
TIME_DELAY[water,coord1]=
    40 ~~|
TIME_DELAY[oil,coord2]=
    0 ~~|
TIME_DELAY[water,coord2]=
    5
    ~   
    ~       |
rogersamso commented 1 year ago

OK. I'll evaluate the second option when I find the time (in a few weeks), and be back to you.

In the meantime, it would be wise to add an exception with a more convenient error message (and possibly a suggested workaround) if the time_delay has dimensions.

enekomartinmartinez commented 1 year ago

@rogersamso would you like to add the exception error message in dev branch?

rogersamso commented 1 year ago

Hi @enekomartinmartinez I don't have the time right now, so if you want to do it yourself, please go ahead.