Skip to content

[10][FIX] Variability factor increase red zone#57

Open
hparfr wants to merge 81 commits intoForgeFlow:10.0from
akretion:fix/variability_factor_increase_red_zone
Open

[10][FIX] Variability factor increase red zone#57
hparfr wants to merge 81 commits intoForgeFlow:10.0from
akretion:fix/variability_factor_increase_red_zone

Conversation

@hparfr
Copy link
Contributor

@hparfr hparfr commented Mar 14, 2018

Instead of reducing it.

Tested manually on minmax buffers.

JordiBForgeFlow and others added 30 commits September 29, 2017 16:52
Copy link
Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Seems make sense.

Weird that tests pass... Please provide a test to ensure result.

@max3903
Copy link
Contributor

max3903 commented Mar 14, 2018

Ping @lreficent

@JordiBForgeFlow JordiBForgeFlow mentioned this pull request Mar 14, 2018
22 tasks
@@ -54,11 +54,11 @@ def _compute_red_zone(self):
if rec.replenish_method in ['replenish', 'min_max']:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to split this, for replenish the code is good. For min_max as we discussed yesterday they changed the computation of red zone on the 2nd edition. Now it is RZ(min_max) = adu * dlt * (1 + variability factor)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to split this, for replenish the code is good

Here I meant the current code without the changes on this PR

"product_uom.rounding", "red_override")
def _compute_red_zone(self):
for rec in self:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hparfr @lreficent Maybe a _get_lead_time_factor() method should be a good approach ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it will make sense now. It can be refactored later when some new buffers will be added. (like time buffer 👯‍♂️ )

Copy link
Contributor

@rousseldenis rousseldenis Mar 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that's what I've in mind 😃

It was to 'decouple' a little bit code!

@LoisRForgeFlow LoisRForgeFlow force-pushed the 10.0 branch 2 times, most recently from d504848 to 4d532f0 Compare July 6, 2018 08:19
JordiBForgeFlow pushed a commit that referenced this pull request Jul 27, 2020
Signed-off-by LoisRForgeFlow
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants