-
Notifications
You must be signed in to change notification settings - Fork 3
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
Calculate total set volume per 7 days #63
Conversation
I think replacing the weekly total with a rolling total over the last 7 days is a good idea. I'm actually not that interested in the already achieved set volume for the current week (i.e. the last value of the weekly total graph), so I wouldn't mind if this info will be omitted. To be consistent, the approach should be applied to all set volume graphs, and a rolling average over the last 7 days should be used for the RPE graphs. I had a look at the current implementation and it seems to be broken. Some days have two values and some days have no value. I guess the current algorithm doesn't take into account that there could be multiple training sessions per day and doesn't calculate values for days without training sessions. |
0aa4b9c
to
4637350
Compare
I changed the set volume, volume load and RPE graphs accordingly to always include the total/average value for the training day as well as a 7 day average for the same day. Please have a look, especially at the RPE graphs as I do not have good data for this (and the demo mode looks a bit artificial to me).
You are right - I fixed the algorithm to combine multiple sessions of the same day now. Do you really think it's necessary to calculate values for days without training sessions? Wouldn't the line that gets drawn between two training days be a good enough approximation? (Or maybe I'm missing something here). The body weight graph could be refactored in a similar fashion. I didn't do that as I don't understand the rationale of the current implementation. What's the reason for the average weight being calculated as a centered average over the value from 4 days ago? Wouldn't the average over the previous 7 days be enough? Feel free to suggest other colors. Two additional minor changes:
|
Unfortunately, the code doesn't work for me:
I didn't find a simple solution for this issue.
The graph can be quite inaccurate if values for days with no training sessions are not calculated. Here is a simple example: Assuming there are two training sessions with 10 sets each on Monday and Friday and no training sessions in the previous 7 days, the rolling total for the last 7 days will be as follows:
If the value for rest days is not calculated, the line would show the following values:
It would also not be nice if it did not show that the total drops after several rest days.
A simple moving average over the previous 7 days lags behind by 3-4 days. Using a centered average avoids this problem and leads to more meaningful values in my opinion. I'm not yet sure if it makes sense to apply the same approach to RPE. |
4637350
to
7756bf0
Compare
I broke the code when refactoring it o make clippy happy and forgot to rerun the tests ;-) This is fixed now.
Thanks for the example - I get the point now and will try that approach.
I don't think I understand. Why would the average of the (values available for the) last 7 days lag behind? |
I think it would be nicer to use a bar chart for the daily total/average and to use yellow as the second color. The result looks as follows: What do you think? Here are the corresponding code changes to the training page.diff --git a/frontend/src/common.rs b/frontend/src/common.rs
index f3ff251..f70ef3d 100644
--- a/frontend/src/common.rs
+++ b/frontend/src/common.rs
@@ -18,9 +18,9 @@ pub const COLOR_LOAD: usize = 1;
pub const COLOR_LONG_TERM_LOAD: usize = 2;
pub const COLOR_LONG_TERM_LOAD_BOUNDS: usize = 13;
pub const COLOR_RPE: usize = 0;
-pub const COLOR_RPE_7DAY: usize = 19;
+pub const COLOR_RPE_7DAY: usize = 2;
pub const COLOR_SET_VOLUME: usize = 3;
-pub const COLOR_SET_VOLUME_7DAY: usize = 19;
+pub const COLOR_SET_VOLUME_7DAY: usize = 2;
pub const COLOR_VOLUME_LOAD: usize = 6;
pub const COLOR_VOLUME_LOAD_7DAY: usize = 19;
pub const COLOR_TUT: usize = 2;
@@ -853,8 +853,8 @@ pub fn plot_bar_chart(
.iter()
.flat_map(|(s, _)| s.iter().map(|(_, y)| *y))
.collect::<Vec<_>>(),
- None,
- None,
+ y_min_opt,
+ y_max_opt,
);
let mut result = String::new();
diff --git a/frontend/src/page/training.rs b/frontend/src/page/training.rs
index 10b5726..2484d68 100644
--- a/frontend/src/page/training.rs
+++ b/frontend/src/page/training.rs
@@ -547,14 +547,12 @@ pub fn view_charts<Ms>(
),
common::view_chart(
&[
- ("Set volume", common::COLOR_SET_VOLUME),
+ ("Set volume (daily total)", common::COLOR_SET_VOLUME),
("Set volume (7 day total)", common::COLOR_SET_VOLUME_7DAY)
],
- common::plot_line_chart(
- &[
- (set_volume, common::COLOR_SET_VOLUME),
- (set_volume_7day_total, common::COLOR_SET_VOLUME_7DAY)
- ],
+ common::plot_bar_chart(
+ &[(set_volume, common::COLOR_SET_VOLUME),],
+ &[(set_volume_7day_total, common::COLOR_SET_VOLUME_7DAY)],
interval.first,
interval.last,
Some(0.),
@@ -566,9 +564,12 @@ pub fn view_charts<Ms>(
IF![
show_rpe =>
common::view_chart(
- &[("RPE", common::COLOR_RPE), ("RPE (7 day average)", common::COLOR_RPE_7DAY)],
- common::plot_line_chart(
- &[(rpe, common::COLOR_RPE), (rpe_7day_avg, common::COLOR_RPE_7DAY)],
+ &[("RPE (daily average)", common::COLOR_RPE), ("RPE (7 day average)", common::COLOR_RPE_7DAY)],
+ common::plot_bar_chart(
+ &[(rpe, common::COLOR_RPE)],
+ &[(rpe_7day_avg, common::COLOR_RPE_7DAY)],
interval.first,
interval.last,
Some(5.), |
The bar chart idea is great! I changed the plotting function to use the same domain for the y-asis as with two axes the 7-day plots are (IMHO) less useful. I find the yellow lines rather hard to see on white background. I don't have a strong opinion, though, as I use dark mode mostly. |
f219906
to
3a4cd35
Compare
The idea is implemented now - please have a look. There are a few things that I'd still like to look into:
|
On the training page using a 7-day rolling total/average looks good to me. I'm not sure how useful the bars with the daily average/daily total actually are. Especially in the RPE chart I have the feeling that the bars for the daily average don't add meaningful information. So I would suggest to at least remove them in the RPE chart. Threre is also bug in the RPE chart. If there are two training sessions in one day, the RPE values are summed instead of averaged: For some reason, this also results in too high values for the 7-day average for the entire interval displayed: I'm not convinced about using this new representation for exercises (and also routines). Here is the new representation: In my opinion the old representation looks better and trends are easier to see: Again, the bars in the new representation do not seem very helpful to me. So I would suggest to remove them here. I'm undecided whether we should use a weekly average/total as before or a rolling 7-day average. Looking at both representations, I think there is also a bug in the old representation. The line should go to zero if there hasn't been a training session in a week, which isn't the case.
I agree, yellow isn't optimal. We could try instead using different colors for the lines (as in the old representation) and grey for the bars (in case we want to keep them). |
I must say, looking at real data the bar chars look awful. We should indeed try something else. Another option is to represent single values as dots and the calculated averages/totals as a lines along with it (similarly to how you would plot statistical data): Please also have a look at the body weight, for which I tried the same graph layout. Don't look at the code too closely, though, as I just quickly hacked it. I definitely must clean up the mess later. Some of the other graphs will also be broken due to the change. Some colors will also be off elsewhere.
That was an error on my part - I just used the wrong function (RPE values got added instead of averaged). This should be fixed now - please have a look.
That might be for the same reason - can you please check this again? My values looked good.
Yep. Let me know what your feeling is about the current suggestions. For exercises I find it pretty helpful now (at least for my real world data). Not sure about the set volumes in routines - my routines usually are 7 days apart such that the 7 day total matches the single values...
For all charts or just the exercises? In general, I have my feeling is that the 7 day average looks a bit smoother.
I believe I fixed that. My most recent change makes the y-values of type
That's what I tried (for the dots). |
The dots look much better than the bars! I still don't see the value in showing the daily total/average in the charts. So I would rather remove these values for the set volume, volume load and RPE charts. If you disagree, it would be okay to me to keep them, as the dots don't detract from the actual interesting information. In my opinion using dots for the short-term load makes the chart worse. Especially for longer intervals it's just a cloud of dots and the actual course of the short-term load isn't visible. Using dots for the long-term load might work better, but I guess a line would still be better. I would also prefer to continue using green for the short-term load.
I like that the long term changes are better visible in the new layout. Having no dots on the line for the average also looks better. I see the drawback that the actual short-term changes are less visible, which at least one other user I've just spoken to doesn't think is so great. So I'm slightly in favor of keeping to use a line instead of dots. For the body weight chart, I would prefer if there were no gaps if there was no value in 7 days. I think it's not that uncommon for a user to miss tracking their weight for a week. Previously there was a small padding on the y-axis to prevent dots from being drawn directly on the axis. Has this been removed on purpose?
Yes, this looks good now. 👍
As I typically only do an exercise once a week, the dots with the daily total/average are completely useless in my case: There are also a lot of gaps, which don't look very nice. Probably because there is some variance in my training days and thus I don't do the same exercise exactly every 7 days. For routines there is the same problem.
The 7-day average/total looks good on the training page. The lines are smoother than before. While the new representation works well on the training page, because there is usually sufficient data, it seems not optimal to me on the exercise and routine page. There are a lot of steps and gaps, as there is often only one data point or no data in a 7-day interval (as in my example shown above).
I think it depends on the type of data whether there should be a gap or a line at 0. If it is an average like for RPE, dropping to 0 wouldn't make sense. The average RPE isn't 0 if you didn't log any set with an RPE. For totals like set volume, having a line at 0 would make sense. If you didn't do any sets, the set volume for this interval is 0. I think it's great that you're trying to find better representations. I hope my complaints don't discourage you. |
Thanks for the feedback. As the discussion got a bit longer, here is a summary of what I'm planning to implement now:
Can you see anything I've dropped or misunderstood from the previous discussion? Per-exercise volume loadHaving an average here in addition to the daily entries has indeed value for me to better identify a trend. I do a lot of undulating exercises which make the single value graph rather useless for that purpose. I just tried an average over 31 days and that produces a pretty helpful trend in my case. Do you think that would be worth trying? I could also imagine scaling the averaging window depending on the interval shown (e.g. don't show averages for 1M, show 15 day average for 3M, show 31 day average for 6M ...). |
For the 7 day calculations I would rather say:
I think the 7-day average calculation doesn't make sense for the body weight chart. If there is only one entry per week, or even less, there will be no average. It's not uncommon to only weigh once a week. The current algorithm adapts to the interval between weight measurements as it always looks at 9 consecutive entries. This is probably the better approach to cover different usage patterns.
Do you see an advantage of plotting the daily set volume here? This would be inconsistent with what is done on the training page as per your point above.
I think making the average dependend on the shown time interval doesn't solve the problem. The problem of the fixed X-day average is that the resulting average is only useful if there is more than one data point per X days. We could try to solve the problem by using the same approach that is currently used for the body weight average, which is to compute a centered rolling average of a fixed number of values. What do you think? |
1d1bba5
to
ac7d9ab
Compare
I could add this. IMHO having a straight line instead of a gap looks OK, too. What do you think?
That's what I meant to say and what I implemented.
I'm not sure how much sense it makes to include a weight that was recorded a couple of months ago (if records are only that infrequent) into the average calculation. The same is true for the per exercise volume load. There should probably be some cap on how old values can be (relative to the center) to be included into the average calculation to make sense. I'd leave this for another discussion and not change either of those charts in this PR.
No, that's not useful. I removed that. |
I think a straight line is also fine. I just have the feeling that a gap would be a better representation of "no data" in that case. Would it be difficult to add this so I can see how it looks with my data?
With the current version there is a straight line, although the total should be zero:
You are probably right, an upper limit for unreasonably long intervals between values seems reasonable. The current plan is to just change the set volume and RPE charts on the training and muscle page in this PR, right? For consistency, I would suggest to also remove the dots in the short-term/long-term load chart on the training page.
That's indeed a nice alternative. This could be also a good option for several other charts. Using it only for the total set volume charts seems a bit arbitrary to me. I think it would be also nice to have some kind of an area plot for the long-term load chart: Do you think that would be difficult to implement? |
Sure, we could think about using it elsewhere, too.
It's almost possible out of the box: This is in fact a hack. As area plots always start at the y=0, I first plot the upper graph and then the lower graph in background color on top of it. Not great, as you lose the grid and the background is not a perfect match (more hacks would be needed here) and yet another hack would be needed to support light/dark mode. The better solution is probably to implement a custom band graph with plotters Polygon drawing functionality. I'll look into that at some point as I plan to use it for nutrition tracking. |
Then it's better to keep using lines until we have a custom implementation. |
ac7d9ab
to
853b327
Compare
Implementing this is not that hard. Have a look at this branch.
The total volume graph was totally wrong as it used the average calculation 🤦 This should work as expected now, please have a look.
Correct.
Done. |
Looks good to me. I would prefer this solution.
Now it looks correct.
👍 The only remaining issue I see is the one mentioned in #64 (comment). I will have another look at the code after #64 is merged. |
853b327
to
725c02b
Compare
I included the gap-related change and rebased this branch onto #64. Btw, I also rebased and updated the area plot branch to improve it visually and apply it to all volume charts. Let me know what you think and whether we want to include it in this PR. |
Looks good! I just think that we should use the same presentation also for other charts to get a consistent look. It could probably be applied to all other training-related charts, except the short-term/long-term load chart. I would suggest creating a separate PR for this change so that this PR is not delayed any further. |
725c02b
to
8d4c695
Compare
8b86b87
to
561bebe
Compare
561bebe
to
6fa81b7
Compare
I'd like to discuss a potential change to how total set volume is represented in the "Set volume (weekly total)" is represented. Currently, this calculated on a per-week basis starting on Monday. IMHO this makes the last value of the graph less helpful, as it will only be meaningful a the end of the (training) week.
What do you think about a 7 day rolling average instead? I changed the graph in this draft PR, but it could also be added in addition to the current graph. Alternatively, the set volume for he current week could be either added as a single value / horizontal line or as a value to the training overview in the main dialog (where low/optimal/high load is shown).
The same idea could also be applied to the per-muscle set volume graphs...