-
Notifications
You must be signed in to change notification settings - Fork 732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Medical - Change max medication dosage behavior #9746
Conversation
And a possible CASE 4 could be to get the random value from a new setting if the ACE Team wants to keep this functionality, but add an opt-out / opt-in option. |
Case 2 makes the most sense to me. |
I agree, although I'm a bit hesitant. Should we adjust the |
John beat me to it. But should I update the other medication's And just out of curiosity, why do you prefer CASE 2 over CASE 3? |
Because medical should be a bit varied, it's more "natural". |
Overdose behavior: ODs on uses `_maxDose + {1 or 2 or 3}` from `_maxDose + {0 or 1 or 2}`
So Ive updated the code to CASE 2. |
I feel like it's clearer as to what the property does. Max implies a limit above which it's no longer safe, as opposed to being the actual point at which it's not safe. @kymckay got any insight on this one? |
It's worth doing so, semantics are an important part of code!
Thinking about it, if anyone were so-inclined this system could probably be further generalised to allow different effects to occur at different dosages - with a randomness distribution configured at each dosage trigger per-medication. |
I was thinking of adding a config parameter with variability parameters, but as you said, out of the scope of this PR. |
I wouldn't say it's per-say out of scope of this PR. This is a simple PR right now, but it may be worth tackling this subject as a whole right here (name change required). I'd be happy to update this. And off course, I'm ready to update the current medications (reducing max dose by 1) to keep their behavior in line with their current limits. Mostly, I started this PR as a conversation starter, but didn't want to do too much without gauging yalls interest.
This is definitely out of scope though. I'm not looking to change the behavior of the current drugs. Edit: New commit coming soon with updated code to medications for adding a overdoseRange and adjusting for the new maxDose. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before anyone commits anything, I want more input for the other ACE devs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good!
Though this comment is still misleading:
The chance increases as dose gets higher
The chance of each dose causing overdose is uniform between the lower and upper dose limits. We're rolling the same die each time, the weighting doesn't change until you hit that upper limit and it becomes 100% weighted to overdose.
Comment has been removed, the example provided in the code as a comment should provide a clear picture of how to use it in any case. |
**When merged this pull request will:** - With the recent ace update, overdose calculations were changed to be variable, with a chance to suffer an overdose based on the max dose deviation acemod/ACE3#9746 - with this PR it will change the default deviation of 2 to more realistic values depending on the medication, eg, fentanyl is now a 1, so any dose over 2 will send then into OD ### IMPORTANT - [Development Guidelines](https://ace3.acemod.org/wiki/development/) are read, understood and applied. - Title of this PR uses our standard template `Component - Add|Fix|Improve|Change|Make|Remove {changes}`.
Goal
Context
When testing a drug mod I'm making, I was getting inconsistent behavior when testing the overdose limit.
With my drug, the maxDose is set to 3. I expect the max safe dosage to be 3. On the 4th use, I OD.
The logic for the original code is this
_currentDose >= floor (_maxDosage + round(random(2)))
The maxDose can be calculated as
_maxDose + {0 or 1 or 2}
.Assuming the random value added is 0, you will overdose on the 3rd use.
Assuming the random value is 2, its possible to exceed the maxDose.
This behavior is confusing.
Edit 1/21/24:
Current agreement is that the behavior of the overdose logic is changed to:
_currentDose > floor (_maxDosage + round(random(2)))
Behavior:
Drug's
maxDose = 3
Overdose possible on dose number
_maxDose + {1 or 2 or 3}
Overdose guaranteed on
_maxDose + 3
and greaterIn my commit, there are 3 cases.### CASE 1 | LINE 29this is the original behavior. Left in there for now for code comparison
### CASE 2 | LINE 30 (This is the same behavior as case 1, but with
+ 1
to raise the randomly added value from {0,1,2} to {1,2,3}.This will at least make the maxDose consistent with the way the maxDose config is understood (at least by me).
This still keeps the random value, possibly allowing for 2 extra doses before ODing.
### CASE 3 | LINE 31Removed the randomly added value and changed the
>=
to>
.If maxDose is set to 3, then the an OD will happen on the 4th dose.