LarrySnyder / stockpyl

Python inventory optimization and simulation tools.
GNU General Public License v3.0
87 stars 19 forks source link

`DemandSource` setting of `standard_deviation` upon instantiation #130

Closed finnwiz closed 3 weeks ago

finnwiz commented 6 months ago

Hi Larry,

Firstly, thank you so much for creating this fantastic library!

Secondly, if you'd like some part-time help, I'd be be happy to contribute an occasional PR, although I expect I would be more useful in some of the code-structuring or testing than in any of the actual optimization / simulation.

Lastly, I've noticed a bug that affects DemandSource. When using optimize_committed_service_times(), I realized that the Poisson distribution I was passing as a demand source didn't actually have its standard deviation set.

a,b,c = DemandSource(type='P',mean=mu_rh),DemandSource(type='P',mean=mu_c1),DemandSource(type='P',mean=mu_c2)
# a.standard_deviation = mu_rh
# b.standard_deviation = mu_c1
# c.standard_deviation = mu_c2

ex_network = network_from_edges(
    edges=[(1,2), (1,3)],
    node_order_in_lists=[1, 2, 3],
    processing_time=[1,0,0],
    external_inbound_cst=[ITEM_LEAD_TIME_FROM_MANUFACTURING, None, None],
    local_holding_cost=[ITEM_HOLDING_COST]*3,
    demand_bound_constant=[db_rh, db_c1, db_c2],
    external_outbound_cst=[None, 3, 3],
    demand_source=[a,b,c],
)
for node in ex_network.nodes:
    print(f"Node {node.index}: h = {node.local_holding_cost} p = {node.stockout_cost} mu = {node.demand_source.mean}  sigma = {node.demand_source.standard_deviation}")

optimize_committed_service_times(ex_network)

Output:

Node 1: h = 5.0 p = None mu = 10  sigma = None
Node 2: h = 5.0 p = None mu = 4  sigma = None
Node 3: h = 5.0 p = None mu = 1  sigma = None
({1: 0, 2: 0, 3: 0}, 0.0)

But if I force the standard deviations to be set, I get a non-zero result.

a,b,c = DemandSource(type='P',mean=mu_rh),DemandSource(type='P',mean=mu_c1),DemandSource(type='P',mean=mu_c2)
a.standard_deviation = mu_rh
b.standard_deviation = mu_c1
c.standard_deviation = mu_c2

ex_network = network_from_edges(
    edges=[(1,2), (1,3)],
    node_order_in_lists=[1, 2, 3],
    processing_time=[1,0,0],
    external_inbound_cst=[ITEM_LEAD_TIME_FROM_MANUFACTURING, None, None],
    local_holding_cost=[ITEM_HOLDING_COST]*3,
    demand_bound_constant=[db_rh, db_c1, db_c2],
    external_outbound_cst=[None, 3, 3],
    demand_source=[a,b,c],
)
for node in ex_network.nodes:
    print(f"Node {node.index}: h = {node.local_holding_cost} p = {node.stockout_cost} mu = {node.demand_source.mean}  sigma = {node.demand_source.standard_deviation}")

optimize_committed_service_times(ex_network)

Output:

Node 1: h = 5.0 p = None mu = 10  sigma = 10
Node 2: h = 5.0 p = None mu = 4  sigma = 4
Node 3: h = 5.0 p = None mu = 1  sigma = 1
({1: 6, 2: 3, 3: 3}, 303.1088913245535)

I'm not sure if you intended generic demand sources to be used in the gsm_tree module (although I didn't see that limitation strictly in the documentation) - but it seems like, within demand_source.py, you could just leverage some of SciPy's properties for the distribution you're constructing.

For example, in lieu of keeping private instance variables in the DemandSource class, you could just do something like:

    @property
    def standard_deviation(self):
        return self.demand_distribution.std

See https://docs.scipy.org/doc/scipy/reference/generated/scipy.stats.rv_discrete.std.html#scipy.stats.rv_discrete.std.

Welcome feedback either way and thank you again for contributing this great library!

LarrySnyder commented 6 months ago

@finnwiz Thanks for this, and I'm glad you're enjoying the package. I'd love some part-time help, if you're willing -- feel free to tackle an open issue or propose your own. Happy to discuss more.

So there's a few things going on in the issue you're describing. First, for a Poisson distribution, the standard deviation always equals the square root of the mean, so there's no need to set the SD field independently -- the DemandSource only needs the mean.

But more importantly, the GSM code doesn't handle anything other than normally distributed demand. So you could set the standard_deviation and it would avoid the bug, but since the safety-stock calculation assumes normally distributed demand, it wouldn't reflect the Poisson-ness of your random variable at all. (Actually I don't think it would be too hard to handle non-normal demands; see also issue #20.)

So I'm going to

And I'll leave handling non-normal demand to fix one day in #20.