Incorrect profile in video chart

[ghost]

I'm using Developer build (build id: 4001). I created gradient workout in GC workout creator. But when i started my workout, last part (i think 1km) is displayed incorrectly.

It is visible on image. At the bottom is actual profile and in video chart you can see it starts to descend. Last part where is descending is 5.7% (positive) http://89.212.19.139/profile_in_video.png

But when i used original (demo) workout file + video + rlv, profile is displayed correctly (in video chart).

BTW, i really like new and improved GC. Great work!

[ericchristoffersen]

Are you talking about riding a crs file? Did you create it in workout creator by importing an activity? Can you please share the activity you imported and the resulting workout file?

I just tried importing a workout into 'workout creator' and created a crs. Seemed to work ok. Crs files are run in 'strict slope mode' meaning the slope in the file is the slope you should get, so you should see crisp boundaries between gradients.

You can ride an exported json file directly which will do a smoothly interpolated slope for the altitude changes.

[ghost]

I didn't use any activity to create crs file. I used Gradient option in workout creator, just to test it. Because last time i used it, it was limiting gradient to max 5% if i'm right (i think in 3.4 version).

crs was created and used in Dev build June 2020. Didn't try it in September build yet.

Crs file is very small, so i will just paste data.

[COURSE HEADER]
VERSION = 2
UNITS = METRIC
DESCRIPTION = golden cheetah
FILE NAME = F:/GC/10k_5-7max.crs
DISTANCE GRADE WIND
[END COURSE HEADER]
[COURSE DATA]
1 0.5 0
2 1.2 0
2.1 3.2 0
1.9 2.7 0
1.7 2.3 0
2.3 4.3 0
0.8 3.4 0
1.2 5.7 0
[END COURSE DATA]

[ericchristoffersen]

I didn't use any activity to create crs file. I used Gradient option in workout creator, just to test it. Because last time i used it, it was limiting gradient to max 5% if i'm right (i think in 3.4 version).

crs was created and used in Dev build June 2020. Didn't try it in September build yet.

Crs file is very small, so i will just paste data.

[COURSE HEADER]
VERSION = 2
UNITS = METRIC
DESCRIPTION = golden cheetah
FILE NAME = F:/GC/10k_5-7max.crs
DISTANCE GRADE WIND
[END COURSE HEADER]
[COURSE DATA]
1 0.5 0
2 1.2 0
2.1 3.2 0
1.9 2.7 0
1.7 2.3 0
2.3 4.3 0
0.8 3.4 0
1.2 5.7 0
[END COURSE DATA]

Thank you! Lots of different possibilities, really helps to narrow down your specific case.

As you said, the elevation line is being displayed incorrectly. I don't see this because when I tried to repro I imported an activity with 1000s of points to crs.

Here is the story. There is a box in the display that holds the elevation display. The code that draws the elevation line in that box finishes by drawing one more segment from the end of elevation line to the bottom right corner of the box... I don't know why that segment is being drawn but that bad segment is the one to the bottom right. So, the good news is that the decreasing slope is a display artifact, actual workout isn't affected. I could just not draw that last line segment but I think is better is to fix so the elevation line is properly scaled to reach the right side of the box.

[ericchristoffersen]

The nested loops that conspired to draw the elevation line were pretty hard to read. Turns out it was always skipping the segment to the final point. Rewrote in a simpler form and it looks good. I'll create pull request soon.

[ericchristoffersen]

Hi Ale. The rewritten code is now very simple. I went to create pull and find I already made some fixes to elevation widget, it no longer needs to loop over ergfile to find min and max. That change is part of #3442 .Can you let me know if that change will go in so I can build off it?

[ericchristoffersen]

@amtriathlon : fix is now pushed into #3442 .

[amtriathlon]

That change is part of #3442 .Can you let me know if that change will go in so I can build off it?

No, I can’t. That PR has a pending review request from @liversedge, he is the project leader and I have no intention to override his decision.

If you want my opinion, I prefer to keep fixes and features in separate commits and PRs, this is easier to verify and allows for easier debugging and maintenance.

Code refactoring and reformatting should be kept separate as explained in https://github.com/GoldenCheetah/GoldenCheetah/wiki/Guidelines-for-submitting-a-patch

[ericchristoffersen]

Sorry I didn't realize that it was waiting for mark. Totally agree that fixes should separate but this stuff is all interwoven and just getting worse. Either I make progress or I'll need to stop. Too many pans on the stove. Or maybe I'm just really bad at git.

I'll ping mark for feedback. Hopefully he can let me know what he wants. I just don't see an alternative to pulling runtime state out of the data files, which is the thing everything else depends on.

[amtriathlon]

Sorry I didn't realize that it was waiting for mark. Totally agree that fixes should separate but this stuff is all interwoven and just getting worse. Either I make progress or I'll need to stop. Too many pans on the stove. Or maybe I'm just really bad at git.

Hi Eric, I could try to cherry-pick the commits fixing specific issues from that PR and apply them to master so we can close the issues they fix, then you can rebase the PR to contain only the refactoring for reviewing.

[ericchristoffersen]

How about I extract the stuff that can be separated. Give me a few days. One problem is that the fixes rely on the data separation.

How do I get a response from Mark about separating the ergfile and videofile data from the training state? Does he have a problem with the core concept? Or is there something specific about what I did that he doesn't like?

[amtriathlon]

No problem, I was trying to help with the separation and referring mainly to #3498, #3499, #3582 and #3589 which look clearly independent, not sure about #3580 and this one which may need some adaptation.

WRT the second part I can only reiterate my former opinion: let's keep refactorings, non-functional code additions and code reformattings separate from bug fixes and new features, adding up makes PRs less likely to be merged, not more.