Skip to content
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

Merged
merged 17 commits into from
Feb 13, 2024

Conversation

BAXENdev
Copy link
Contributor

@BAXENdev BAXENdev commented Jan 19, 2024

Goal

  • Update maxDose to be "max safe dose"
  • Update current medications to be consistent with new maxDose behavior
  • Add new "overdoseRange" attribute to medications

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 greater

In my commit, there are 3 cases.
### CASE 1 | LINE 29
this 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 31
Removed the randomly added value and changed the >= to >.
If maxDose is set to 3, then the an OD will happen on the 4th dose.

@BAXENdev
Copy link
Contributor Author

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.

@LinkIsGrim LinkIsGrim changed the title Ace med max dose normalized Medical - Change max medication dosage behavior Jan 19, 2024
@LinkIsGrim LinkIsGrim added the kind/change Release Notes: **CHANGED:** label Jan 19, 2024
@LinkIsGrim
Copy link
Contributor

Case 2 makes the most sense to me.

@johnb432
Copy link
Contributor

Case 2 makes the most sense to me.

I agree, although I'm a bit hesitant. Should we adjust the maxDose for all medication or leave them be?

@BAXENdev
Copy link
Contributor Author

BAXENdev commented Jan 19, 2024

Case 2 makes the most sense to me.

John beat me to it. But should I update the other medication's maxDoses to keep them consistent?

And just out of curiosity, why do you prefer CASE 2 over CASE 3?

@johnb432
Copy link
Contributor

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}`
@BAXENdev
Copy link
Contributor Author

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".

So Ive updated the code to CASE 2.
Practically, I can achieve my desired effect by raising max dose by 1.
Essentially, I'm arguing over semantics at this point.
I think there is some value in updating this behavior for semantics/clarity, but also the workaround is really easy. So is this worth updating?

@LinkIsGrim
Copy link
Contributor

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?

@kymckay
Copy link
Member

kymckay commented Jan 21, 2024

Essentially, I'm arguing over semantics at this point.

It's worth doing so, semantics are an important part of code!

  • I agree with the change to > because it's intuitive to interpret "max dose" as "max safe dose".
  • We should probably bump (but also review) the existing medication configs for consistency with old behaviour.
  • On the topic of randomness, we should perhaps provide a way to configure the max limit as a strict max limit (I don't think that has to be done in the scope of this PR). The idea of randomness it to simulate varying tolerances / physiological differences in individuals, but there probably are medications where it can be strictly said "If you take 4, you will die".
  • The way we're distributing the randomness here isn't justified in the code and could perhaps be improved. With this logic patients will take 1 over the max 50% of the time and 2 over the max 25% of the time - that may not be intuitive.

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.

@johnb432
Copy link
Contributor

Essentially, I'm arguing over semantics at this point.

It's worth doing so, semantics are an important part of code!

* I agree with the change to `>` because it's intuitive to interpret "max dose" as "max safe dose".

* We should probably bump (but also review) the existing medication configs for consistency with old behaviour.

* On the topic of randomness, we should perhaps provide a way to configure the max limit as a strict max limit (I don't think that has to be done in the scope of this PR). The idea of randomness it to simulate varying tolerances / physiological differences in individuals, but there probably are medications where it can be strictly said "If you take 4, you will die".

* The way we're distributing the randomness here isn't justified in the code and could perhaps be improved. With this logic patients will take 1 over the max 50% of the time and 2 over the max 25% of the time - that may not be intuitive.

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.

@BAXENdev
Copy link
Contributor Author

BAXENdev commented Jan 21, 2024

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.

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.

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.

Copy link
Contributor

@johnb432 johnb432 left a 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.

addons/medical_treatment/ACE_Medical_Treatment.hpp Outdated Show resolved Hide resolved
addons/medical_treatment/functions/fnc_medicationLocal.sqf Outdated Show resolved Hide resolved
addons/medical_treatment/ACE_Medical_Treatment.hpp Outdated Show resolved Hide resolved
addons/medical_treatment/ACE_Medical_Treatment.hpp Outdated Show resolved Hide resolved
addons/medical_treatment/ACE_Medical_Treatment.hpp Outdated Show resolved Hide resolved
@LinkIsGrim LinkIsGrim added this to the Ongoing milestone Feb 12, 2024
Copy link
Contributor

@LinkIsGrim LinkIsGrim left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@kymckay kymckay left a 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.

@johnb432
Copy link
Contributor

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.

@johnb432 johnb432 merged commit a0f3933 into acemod:master Feb 13, 2024
5 checks passed
MiszczuZPolski pushed a commit to KAT-Advanced-Medical/KAM that referenced this pull request Apr 15, 2024
**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}`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/change Release Notes: **CHANGED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants