[PATCH] Revised Intervals Patch

[Mark Liversedge]

Hi,

Attached is a revised version of the intervals patch that should apply
cleanly to the current head.

As before this patch;
* Adds User Definable Intervals on AllPlot using shift-Left Click
* Adds WKO+ Interval reading (but hides the markers on AllPlot as they
look ugly)
* Adds a Find Peak Powers menu option
* Adds an add to intervals button to the Find Best Intervals Dialog
* Provides an Interval selector on the left of the screen (with
right-click context sensitive menus to rename, delete and zoom)
* Makes the AllPlot, Histogram and PfPv Charts all Interval aware
* Introduces the new GC File format and a Save option (with a conversion
warning)
* Checks for save of changed files on exit

And I have changed it to follow better practices by;
* Implementing intervalsChanged and intervalSelected signals from
MainWindow in all the charts and plots
* All "jankey code" to access private vars have been removed (except
IntervalPlotData is a friend of AllPlot)

I fixed up;
* Not showing interval summaries when only 1 interval defined
* RideSummary does not re-read the ride file if it is already loaded

Note:
* All Damien's code was perfect, the kludges were mine
* We might want to reverse the left-click to zoom and shift-left-click
to select on AllPlot (?)
* The GC file format does not support GPS data currently, so when Greg
Lonnon's Map patch gets published we will need to address that

Enjoy!
Mark

[Jamie Kimberley]

Mark,

Please reformat the patch using git format-patch.

Jamie

> --
> _______________________________________________
> Golden-Cheetah-Users mailing list
> golden-che...@googlegroups.com
> http://groups.google.com/group/golden-cheetah-users?hl=en

"It is a mistake to think you can solve any major problems with just
potatoes."--Douglas Adams
__________________
Jamie Kimberley
Postdoctoral Fellow
Department of Mechanical Engineering
The Johns Hopkins University
Office: 410.516.5162
Mobile: 217.621.8272
Fax: 410.516.4316
E-Mail:jamie.k...@jhu.edu

[Mark Liversedge]

Oops, good point Jamie!

Attached in git format-patch style, applies cleanly against current HEAD
(2db45dc).

Regards,
Mark

[Justin F. Knotzke]

2009/12/12 Mark Liversedge <liver...@gmail.com>

Hi,

Attached is a revised version of the intervals patch that should apply
cleanly to the current head.

   It does and I've been playing with it now for a while and HOLY ! Awesome ! That PF/PV interval selection is brilliant !

   Fantastic stuff. This, IMHO is one of the more requested features I see from non GC users. We've closed the gap and surpassed it.

   Very cool.

   J 

[Sean Rhea]

Sweet awesomeness!

I can't wait for this patch any longer, so I've committed it. That
said, I think there are a bunch of little things that need to be
fixed. I can do these, or you can. Let me know which you'd prefer:

1. There's a rule in C++ that the first include in a .cpp file should
be the corresponding .h file. I can't remember why, but it's in Scott
Meyer's book. You added MainWindow.h before AllPlot.h in AllPlot.cpp.

2. Same file: why did you remove this line:

if (interval.start < xaxis[startingIndex])

3. Does anything ever call delete intervalPlotData?

4. I think you can replace this line:

mainwindow->allRideItems()->treeWidget()->selectedItems().first();

with this one

mainwindow->rideItem();

5. in clearResultsTable, ending a function with "return;" is JV. Same
for BestIntervalDialog::addClicked.

6. Also in addClicked, why the cast to (RideFile*)? Coding
conventions say to avoid C-style casts, but I'm also not sure you need
it at all.

7. The copyright lines in IntervalItem.h|cpp are probably wrong. I
don't think I wrote them. :)

8. In MainWindow.cpp, I think intervalWidget needs a parent pointer in
its constructor.

9. I don't understand the comment about disconnecting the signal from
intervalWidget.

10. Why the commented-out code in updateRideFileIntervals?

11. I don't understand MainWindow::itemChanged at all.

12. Can we use std::lower_bound in RideFile::distanceToTime and/or
RideFile::timeToDistance? Maybe we need an assertion that
time/distance only go forward?

13. Does MainWindow::saveRideSingleDialog need to call delete dialog?
Or better yet, why not allocate it on the stack? Same comment for
MainWindow::saveRideExitDialog.

14. Why the commented out lines in MainWindow::saveSilent?

15. In WkoFileReader::openRideFile, when you call references.clear(),
shouldn't you delete each of its elements first?


Again, huge congrats to you and Damien. This is an awesome piece of work!

Sean

> --
> _______________________________________________
> Golden-Cheetah-Users mailing list
> golden-che...@googlegroups.com
> http://groups.google.com/group/golden-cheetah-users?hl=en

--
"I refuse to accept the idea that the ‘isness’ of man’s present nature
makes him morally incapable of reaching up for the eternal ‘oughtness’
that forever confronts him." --MLK

[Eric Murray]

This is so cool! Thanks so much for doing this. It's a huge addition.


I have a few small suggestions-

-make the new selection appear in the intervals list between the last existing
interval and the first peak powers interval.

-add the avg watts to the new interval's name in the intervals list. i.e.
Selection #1 (256 watts)
and perhaps to the named interval names as well?

and a small bug-

-the selection shading in the graph doesn't handle overlapping areas-
i.e. if the best 10 minutes and best 20 minutes both start at the same point
when you click on the best 10 and then the best 20, the last 10 of
the best 20 are highlighted and the first 10 go back the the background color.


A not so small suggestion is to make it easier to change the boundaries
of a user defined interval (and maybe an interval from the computer?).
Maybe key bindings to move each boundary a small amount at a time, or
an easy way to zoom in and then be able to drag it. It's difficult to
get it positioned exactly right and if you don't you have to delete it
and start over.


Also, this brings up limitations in the existing Allplot graph.
It wastes space vertically yet is is crowded horizontally.
The layout makes it difficult to see any trends.

I hate to say it, but the 'stacked' view in WKO is one good way to solve
the problem. It's the best I have seen but I am no information design expert.
Is there is another way that's better?



Did I mention how awesome this patch is? My wife had to ask what
I was so excited about.


Eric

[Sean Rhea]

Eric,

Can you send a screenshot of the stacked view? I've never used WKO.

Sean

[robert carlsen]

This is really great! Thanks for all the hard work.
Have we addressed the file size inflation with the xml format? Are we
thinking about gzip compression?

-robert

[Eric Murray]

On Sat, Dec 12, 2009 at 12:18:29PM -0800, Sean Rhea wrote:
> Eric,
>
> Can you send a screenshot of the stacked view? I've never used WKO.
>

Sure, attached.

Eric

[Sean Rhea]

Oh, so it's just one separate graph per 15 minutes or so, and then
stack them vertically?

Sean

[robert carlsen]

i believe that the duration of each horizontal graph is dependent on the zoom level and width of the window, FWIR....

...maybe Hunter could confirm that?

[Eric Murray]

On Sat, Dec 12, 2009 at 04:54:24PM -0500, robert carlsen wrote:
> i believe that the duration of each horizontal graph is dependent on the zoom level and width of the window, FWIR....

Yep.

Eric

[Mark Liversedge]

On Dec 12, 7:44 pm, Eric Murray <er...@lne.com> wrote:
> This is so cool!  Thanks so much for doing this.  It's a huge addition.

I know I keep saying it, but all the cool stuff is Damien's work!

> I have a few small suggestions-
>
> -make the new selection appear in the intervals list between the last existing
> interval and the first peak powers interval.

>
> -add the avg watts to the new interval's name in the intervals list. i.e.
> Selection #1  (256 watts)
> and perhaps to the named interval names as well?

Will look at that, thanks for the ideas.

> and a small bug-
> -the selection shading in the graph doesn't handle overlapping areas-
> i.e. if the best 10 minutes and best 20 minutes both start at the same point
> when you click on the best 10 and then the best 20, the last 10 of
> the best 20 are highlighted and the first 10 go back the the background color.

Oh, I kind of like that. I guess I could look at changing it tho, I
wonder if it would be better to color each interval differently so the
overlapping was more visible?

> A not so small suggestion is to make it easier to change the boundaries
> of a user defined interval (and maybe an interval from the computer?).
> Maybe key bindings to move each boundary a  small amount at a time, or
> an easy way to zoom in and then be able to drag it.   It's difficult to
> get it positioned exactly right and if you don't you have to delete it
> and start over.

That is tricky!

> Also, this brings up limitations in the existing Allplot graph.  
> It wastes space vertically yet is is crowded horizontally.
> The layout makes it difficult to see any trends.
>
> I hate to say it, but the 'stacked' view in WKO is one good way to solve
> the problem.  It's the best I have seen but I am no information design expert.
> Is there is another way that's better?

I kind of like that too, but selecting intervals will be tricky. I
also think a scrollbar on the existing graph and zoom in and out
whilst retaining the same y-axis would be quite nice too. And its
something that's relatively easy to implement - the realtime qwt
example shows how to do this.

> Did I mention how awesome this patch is?  My wife had to ask what
> I was so excited about.

LOL!

Regards,
Mark

[Jamie Kimberley]

On Sat, 12 Dec 2009, Mark Liversedge wrote:

> On Dec 12, 7:44 pm, Eric Murray <er...@lne.com> wrote:
>> and a small bug-
>> -the selection shading in the graph doesn't handle overlapping areas-
>> i.e. if the best 10 minutes and best 20 minutes both start at the same point
>> when you click on the best 10 and then the best 20, the last 10 of
>> the best 20 are highlighted and the first 10 go back the the background color.
>
> Oh, I kind of like that. I guess I could look at changing it tho, I
> wonder if it would be better to color each interval differently so the
> overlapping was more visible?

Mark/Damien, Could you have the alpha values (transparency) add in
overlapped areas? That would seem like a logical way to show
overlapping intervals. This would probably look good for an interval
contained completely within another interval but it might look like
there is an "interval" when two intervals overlap at the end of one
start of another. Does that make sense. I think I'd need to see it
and try different interval selections to see how it looks.

Jamie

[Gary Smith]

I built the latest version after commit 3c72bel on linux (Debian) and I
got the following bugs/crashes on the firsts run

a) I tried to "Find best intervals" and I was told that "Ride not
selected" or something similar
b) Then I tried to "Find power peaks" and GC crashed.

Before that I had cycled thru the tabs so I think it has something to do
with the recently implemented signalling technique.

I re-started and went thru the same process but I can't replicate the
problem. If I do I'll be sure to let you know.

[Diarmuid]

Was just about to post about the same problem.

On a Mac (OSX 10.5 PowerPC) and built from 3c72be18ffe

Exact same problems which corrected themselves after restarting from
the crash.

[Sean Rhea]

Hmm. I haven't seen anything, but I'll poke around some more. If
either of you find a reproducible procedure for causing a crash,
please let me know.

Sean

[Diarmuid]

I rolled back both the stress.cache and the plist to prior to the
upgrade and tried to replicate the bug. Failed.

I'll keep an eye out for it

[Mark Liversedge]

On Dec 12, 8:48 pm, robert carlsen <rob...@robertcarlsen.net> wrote:
> This is really great! Thanks for all the hard work.
> Have we addressed the file size inflation with the xml format? Are we  
> thinking about gzip compression?

Do you think this should be user selectable?

As much as the WKO format is not very accessible it does read really
fast. I've been slowly converting my files to GC XML format and it is
quite noticeably slower to read ride files greater than 3 or 4 hours
long with 1 second recording on my dual core Linux box.

If we added compression on top of that it would be even slower!

If we made 'compress ride file data' a user selectable option we could
compress all files regardless of type, or we could have a 'use
compressed GC format' option which only compresses the GC format
files.

I am asking myself -- Is it *really* worth it? I mean, disk space is
cheap?

Regards,
Mark

[Sean Rhea]

Maybe we could add the ability to read compressed files, and then
compress only those files that are older than 1 month? Maybe the best
of both worlds?

Sean

[Justin F. Knotzke]

I share mark's skepticism. The requirement is that gc uses less disk space because the ride files are too large.

   Really ?  Is this really a problem ? Of all the emails to this list,  how many have been because gc is hogging disk space ?

     I dunno.  I don't see it.

      J

On Dec 14, 2009 10:32 AM, "Sean Rhea" <sean....@gmail.com> wrote:

Maybe we could add the ability to read compressed files, and then
compress only those files that are older than 1 month?  Maybe the best
of both worlds?

Sean

On Mon, Dec 14, 2009 at 9:18 AM, Mark Liversedge <liver...@gmail.com> wrote: > On Dec 12, 8:48 ...

-- "I refuse to accept the idea that the ‘isness’ of man’s present nature makes him morally incapab...

Golden-Cheetah-Users mailing list golden-che...@googlegroups.com http://groups.google.com/gro...

[robert carlsen]

well, the new XML file format inflates the ride file by at least a factor of 4 for the several that i've converted.
i just recall the discussion about file size when we first floated the XML format idea.

> --
> _______________________________________________
> Golden-Cheetah-Users mailing list
> golden-che...@googlegroups.com
> http://groups.google.com/group/golden-cheetah-users?hl=en

[Eric Murray]

Extrapolating from the one Xml file I have, I'd wind up with
about 180 megs of data per year if the files were Xml format.
(assuming a 550 hour year). It's about 330kb per hour for a Powertap.

That's not too awful these days.

Eric

[Justin F. Knotzke]

2009/12/14 Eric Murray <er...@lne.com>

   If I have to delete my entire Celine Dion Collection of mp3's to make room for my rides, then that's the kind of sacrifice I am willing to make.

   J ;-) 

[Jim Ley]

2009/12/14 robert carlsen <rob...@robertcarlsen.net>:

> well, the new XML file format inflates the ride file by at least a factor of 4 for the
> several that i've converted. i just recall the discussion about file size when we first
>floated the XML format idea.

As a user, I would prize rapid access ahead of file size any time or
day. And Windows and Mac (since Snow Leopard?) both ship with
compressed filesystem options and I'm sure your average linux user
needs to no assistance finding one, so it doesn't need to be in the
application.

Jim.

[J.T. Conklin]

Mark Liversedge <liver...@gmail.com> writes:
> As much as the WKO format is not very accessible it does read really
> fast. I've been slowly converting my files to GC XML format and it is
> quite noticeably slower to read ride files greater than 3 or 4 hours
> long with 1 second recording on my dual core Linux box.
>
> If we added compression on top of that it would be even slower!

This seems like something we should measure before declaring it to be
so. Some years ago, the firmware image for a device my company made
grew to the point where two images didn't fit on the flash. I added
decompression to the boot loader, and we found that the overall boot
time was significantly reduced even though the additional computation
was required. This was because ~ half the I/O was needed, and it was
I/O, not CPU, that was the bottleneck.

--jtc

--
J.T. Conklin

[Mark Liversedge]

On Dec 14, 9:49 pm, j...@acorntoolworks.com (J.T. Conklin) wrote:

I tried a quick go with the WKO file in the tests directory
2009_07_04_06_09_48.wko, I converted it to .gc and then timed a cat
to /dev/null it took 0.006s including sys and user time. Opening after
that in GC (I timed with a stopwatch) took, roughly, 3.5 secs. Both on
my Macbook Pro, of course there is all the additional overhead of
setting up the charts etc which probably helps to explain some of the
difference. Either way I reckon CPU is the "problem" here not io.

TBH, performance is fine on <3 or 4 hour rides its just those long uns
that take a while.

I took a look at VTD and wondered if it would speed up parsing... I'll
have a play with it and report back.

[David Brower]

Zlib should be our friend; a 72k .wko file becomes a 367k xml file, and
then a 42k compressed file. This is how the XML format in OpenOffice
ODT files is smaller than Word files, and is pretty common practice for
XML structured files. XML is nothing if not redundant and amenable to
compression.

-dB

[Gary Smith]

Sean Rhea wrote:

Hmm.  I haven't seen anything, but I'll poke around some more.  If
either of you find a reproducible procedure for causing a crash,
please let me know.

Sean



On Sun, Dec 13, 2009 at 5:49 AM, Diarmuid
<diarmuid....@googlemail.com> wrote:
  

Was just about to post about the same problem.

On a Mac (OSX 10.5 PowerPC) and built from 3c72be18ffe

Exact same problems which corrected themselves after restarting from
the crash.



On Dec 13, 12:20�pm, Gary Smith <gary.smith....@gmail.com> wrote:
    

I built the latest version after commit �3c72bel on linux (Debian) and I
got the following bugs/crashes on the firsts run

a) I tried to "Find best intervals" and I was told that "Ride not
selected" or something similar
b) Then I tried to "Find power peaks" and GC crashed.

Before that I had cycled thru the tabs so I think it has something to do
with the recently implemented signalling technique.

I re-started and went thru the same process but I can't replicate the
problem. If I do I'll be sure to let you know.
      

--
_______________________________________________
Golden-Cheetah-Users mailing list
golden-che...@googlegroups.com
http://groups.google.com/group/golden-cheetah-users?hl=en
    

I had this error/crash again and think that I have found a clue. I had just downloaded a ride from my PT and was trying to review it. After restarting all was perfect again.

So I suspected that it only occurs on a "freshly" downloaded ride. So to prove this I deleted the ride and downloaded it again. Same procedure different result. This time it worked fine.

But I know that the the original download set a state and/or created a file (entry) that wasn't reset by the "Delete ride" menu action so I wasn't back at the point where I was with the original file download This I know because when I downloaded the ride for the second time GC had remembered the notes that I had entered following the first download.

So I suspect that it is to do with trying to do the "Find best interval" action followed by "Find power peaks" action on newly downloaded file.

I'll only be able to confirm this when I download another ride, but hope that it is enough info to go on.