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

Remove the experimental in Temp target view #154

Closed
avouspierre opened this issue May 6, 2024 · 17 comments
Closed

Remove the experimental in Temp target view #154

avouspierre opened this issue May 6, 2024 · 17 comments
Assignees

Comments

@avouspierre
Copy link
Contributor

Is your feature request related to a problem? Please describe.
The temp target view allows to activate a experimental slider to "simulate" a target in regard of the % basal insulin (HBT) . Now, the implementation is done with Override profil and not useful to stay here
CleanShot 2024-05-06 at 13 41 56

Describe the solution you'd like
remove this part in the view and all code associated in HomeView and others...

Describe alternatives you've considered
no object

Additional context

@bjornoleh
Copy link
Contributor

I agree that the slider is not needed in the Temp target setting if profiles can be used instead. For caregivers, there is still a slight barrier to adopting profiles though, as active profiles/overrides are not displayed in Nightscout. Perhaps we could get #145 sorted first? I don’t know if that is a big or small task.

@avouspierre
Copy link
Contributor Author

I'm working on #145 with a complete refactoring of the code of override management (a PR is in progress) - not the link with oref but the storage in coredata, respect of MVP pattern... -. But the part of this code (experimental) required to be refactor too... so my idea to remove it.

@bjornoleh
Copy link
Contributor

Ok, that’s great! Knowing a refactor of overrides is in the works, for me at least, feel free to remove that slider!

@mountrcg
Copy link
Contributor

mountrcg commented May 14, 2024

I think, substituting temp targets completely with profile overrides amputates an important oref feature. HBexerciseT is crucial to setting meaningful temp targets.
The current implementation is a little backwards in terms of boundaries of the sliders.
Here is how I had implemented it. With a low TT you can only set lower sensitivity meaning more (>100%) insulin. Boundary is autosensMax. With a highTT you can only set a higher sensitivity meaning less (<100%) insulin.

RPReplay_Final1715693903.mp4

@mountrcg
Copy link
Contributor

There is not just Loop users migrating to Trio, but also AAPS users who are familiar with TT and exercise mode. This gives them a great usability and flexibility for TempTargets.

@bjornoleh
Copy link
Contributor

I don't think it was proposed to remove temp targets as such, only the slider. But there might be a case for keeping it still? Your implementation seems to take target and percent insulin as input, and adjust halfBasal target?

@avouspierre
Copy link
Contributor Author

I don't want to remove TT but remove the slider and the logic behind. I don't understand the difference between the experimental slider and the override function. For me, it is a duplicate function and override allows to do more options. I know @mountrcg that you don't implement override profil but Trio has the functionality.

@mountrcg
Copy link
Contributor

Yes I am aware that it is not about removing TTs. But adjusting HBT is what makes TTs flexible as it adjusts the sensitivity change impact for any given TT. So the slider lets you do that quite easily. Its valuable for exercise at different intensities. TT only adjusts sensitivity. Overrides adjust a whole lot of things including CR I think.

@mountrcg
Copy link
Contributor

mountrcg commented May 14, 2024

Your implementation seems to take target and percent insulin as input, and adjust halfBasal target?

yes Jons slider adjusted HBT and the target and calculated the sens ratio. That results in unnatural boundaries for HBT as you need that for the slider.
The slider in autoISF calculates hbt with the input of desired target and sens ratio. Both have easily defined boundaries.
What is left to do, is that the calc result replaces the current hbt setting in profile, while at the moment it injects the result into oref overriding the hbt that comes from the profile - not good.

@bjornoleh
Copy link
Contributor

The CR adjustment is optional in profiles. In addition, SMB can be disabled permanently or on schedule for the duration of the profile/override.

@avouspierre
Copy link
Contributor Author

The difference between temp target with experimental and override is two different algos :
the first will modify HBT before sending to oref. and determinebasal is based on this new HBT profile.

the second did not change hbt but send the "override pourcentage" to oref. oref use the override to recalculate the ISF (and eventually the CR).

So, the result could be different ? Do we have to stay with this 2 algos in Trio ? @MikePlante1 ?

@mountrcg
Copy link
Contributor

Yes indeed 2 different things. TT is short term, especially sports or eating soon stuff where only isf is adjusted, nothing else.
At the moment where you adjust cr, isf, basal etc you are on a not-so-short term adjustment reason. This gets mixed up in the current override implementation because people are used to that from Loop.

@dnzxy
Copy link
Contributor

dnzxy commented May 26, 2024

I‘d vote for fixing the TT slider rather than removing it. There are different types of users and those that use TTs will want to be able to adjust their HBT, the slider is just not a really good implementation to it.

@dnzxy
Copy link
Contributor

dnzxy commented May 26, 2024

The difference between temp target with experimental and override is two different algos

It’s not different algorithms, it’s different inputs that result in different outputs. As long as it’s clearly documented and intuitive to the user what the difference in input and output is (currently I‘d say it’s not), that’s an okay thing to have in the app and would work as expected.

@mountrcg
Copy link
Contributor

mountrcg commented May 26, 2024

refer to mountrcg@7974cc2 as a commit for Trio improving the TT slider.

@bjornoleh bjornoleh removed their assignment Jun 4, 2024
Copy link

github-actions bot commented Jul 7, 2024

hey 👋 - silence for 30 days 🤐 ... anybody? triage is required!

@github-actions github-actions bot added the stale label Jul 7, 2024
Copy link

closed 📴 because silencio 🤫 since an additional 14 days after staleness 📠

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

4 participants