Bug 286642 - Dynamic playlist behaviour still suboptimal when no songs match (even one of the) constraints
Summary: Dynamic playlist behaviour still suboptimal when no songs match (even one of ...
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Playlists/Dynamic Playlists (show other bugs)
Version: 2.6-git
Platform: Compiled Sources Linux
: HI normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks: 292648
  Show dependency treegraph
 
Reported: 2011-11-15 00:57 UTC by Thomas Fjellstrom
Modified: 2013-02-07 15:25 UTC (History)
7 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.8
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Fjellstrom 2011-11-15 00:57:02 UTC
Version:           2.4-GIT (using KDE 4.6.5) 
OS:                Linux

When I setup a dynamic play list the following bias, it picks seemingly random tracks from my entire collection regardless of the other bias's added to the dynamic playlist.

bias: "Last Played older than 7 days"

If I invert the bias, it then starts working.

Reproducible: Always

Steps to Reproduce:
Create a new dynamic playlist with two Match All bias's, for example:

1. genre:Rock
2. lastplay:>7d

Actual Results:  
Songs that don't match the genre restriction are added to the playlist.

Expected Results:  
Only rock songs not played in the last week should be added to the playlist.
Comment 1 Ralf Engels 2011-11-16 17:26:22 UTC
In this case it might actually help to use a "search" bias.
You can click it together in the collection tree view and check if everything works.
Then copy it over to the dynamic playlist.

And it is even faster than two separate biases.

Your problem might be caused by two things:
1. the internal buffers are kept quite long to prevent frequent costly searches over the whole database
2. there is also a "unknown" last played.

So you might get the same songs over and over until you restart Amarok. 
Which is probably something we should fix.

Cheers,
Ralf
Comment 2 Thomas Fjellstrom 2011-11-16 17:56:28 UTC
So you're saying I should use those two conditions in a single search bias? I haven't noticed a problem with duplicate songs, but the amount of music it has to pick from is significant, so it might be quite some time before it'd actually do that. I'll try this soon, but I'm a little busy with renovating my bathroom. I'll reply as soon as I can try it.
Comment 3 Thomas Fjellstrom 2012-01-04 22:57:27 UTC
Just updated to 2.5, and the same thing seems to be happening. I've tried your suggestion to just use a search bias, and even testing it out in the collection search either gives me one song for "genre:genrehere AND lastplay:<4d" or nothing for "genre:genrehere AND lastplay:>4d".

My best guess is that it won't select any songs at all if the db has been cleaned out (which keeps happening to my main collection, last couple times I restarted amarok, I had to rescan).

If that is the case, maybe it would make more sense for the dynamic playlists select /nothing/, or at least match the first condition, rather than random songs.

I've just setup a dynamic playlist that AND's a genre search, and an OR for "lastplay:<4d" and that seems to select only a specific genre. But I won't be able to test if it actually works till my collection builds up a bunch of history again.
Comment 4 Wyatt Epp 2012-04-08 10:23:55 UTC
I can confirm similar behaviour with search constraints.  In my case, I've been using 
Search for: playcount:0 (which works correctly in the collection browser) 
...and things are getting pulled in that have been played inside of a few hours, even.  I saw similar issues with:
Search for: lastplay:>30d

Amarok git 8b0b4012 (2012-03-09)
Comment 5 Mathias Dietrich 2012-06-12 10:43:01 UTC
I can confirm this issue in Amarok 2.5.90.
Comment 6 Myriam Schweingruber 2012-06-15 07:05:22 UTC
Bumping importance as this blocks other issues.
Comment 7 Mayank Madan 2012-11-30 13:19:31 UTC
Still reproducible in v2.6.0-421-gf66a306
Comment 8 Ralf Engels 2012-12-18 18:45:28 UTC
I just tried it and here it works.

Please do the following: Create a new playlist layout to display genre and last played, ensure that you really have selected the dynamic playlist you want (double click it. it has to be in a bold font) and the press repopulate.
Comment 9 Matěj Laitl 2013-01-02 10:00:27 UTC
Reporters, please try with at least v2.6.90-45-g6b19ea3 (post 2.7 Beta), this could be fixed by Ralf's commits.
Comment 10 Mathias Dietrich 2013-02-01 10:16:33 UTC
I rechecked the case with Amarok 2.7.

Ok, the problem is, if your files don't have a first and last play date, the dynamic playlist it breaks your playlist contraints. E.g. when I have set a strict genre (genre:=Dance) and set a last played older constraint (lastPlayed:<01.02.2013) and Amarok hasn't played a file so far, it fills up songs randomly.

In general this behavior is OK, but because there are no files older than the date, also the genre is broken, because I got random songs with random genre.

I don't think it's the optimal behaviour. I would propose to either leave the playlist blank if you constraints do not fit or at least fill it up randomly with the workings constraints.
Comment 11 Matěj Laitl 2013-02-01 10:37:51 UTC
(In reply to comment #10)
> I don't think it's the optimal behaviour. I would propose to either leave
> the playlist blank if you constraints do not fit or at least fill it up
> randomly with the workings constraints.

Roger that.

Ralf, I think we should remove the "fill with random when no song matches" behaviour - that should solve also the other case where the other constraints are ignored when there is not reason to do so.

I think Logger->longMessage() is appropriate to tell the user that no more songs match. (however, we should emit it only when there are really no more songs to be played, not just when we are unable to add one more song to a non-empty play queue)
Comment 12 Ralf Engels 2013-02-01 18:59:28 UTC
Currently we are trying to find a playlist that maximizes the matching of the constraints.

All random playlist with an impossible to match constraint are equally bad.
But what about a constraint with an "AND"? Is a half matching track half good?

When I implemented the code I decided that a track either matches or it doesn't. It's a simplification from the previous dynamic playlist that had less complaints because nobody understood how it worked.

Let me think about it. Maybe we should change the initial requirement to "make the longest playlist (up to 50 tracks) that maximizes the matching of the constraints"
That would allow a playlist with zero tracks.
Comment 13 Matěj Laitl 2013-02-01 20:19:34 UTC
(In reply to comment #12)
> Currently we are trying to find a playlist that maximizes the matching of
> the constraints.
> 
> All random playlist with an impossible to match constraint are equally bad.
> But what about a constraint with an "AND"? Is a half matching track half
> good?
> 
> When I implemented the code I decided that a track either matches or it
> doesn't. It's a simplification from the previous dynamic playlist that had
> less complaints because nobody understood how it worked.

Agreed. Please keep it simple, stupid. When you have "A and B and C" and C doesn't hold, the whole expression doesn't hold. Let the playlist starve in this case (with a message when there's really nothing more to play).
Comment 14 Ralf Engels 2013-02-01 20:30:29 UTC
It's actual a very good example where old design decisions bleed into new implementations.

Why did I do this kind of optimization? I just kept the simulated annealing code intact.
Comment 15 Ralf Engels 2013-02-06 12:35:02 UTC
I have a version that behaves as proposed in comment #13.

However there is one bad/surprising behaviour.
If the current track in the playlist does not have a similar track and the bias looks for similar tracks, then the list just stays empty.

It's better for a rating:>5 search bias that clearly cannot be fulfilled, but it might be confusing for a "similar track" bias where you can't see why it's not working.

btw. if the current track has a similar track, then the list will be filled properly as the algorithm goes back and forth to find a row of tracks that all are similar.

Question: is that behaviour OK? Should the playlist accept "not so similar tracks" in some cases?
Comment 16 Matěj Laitl 2013-02-06 12:49:12 UTC
(In reply to comment #15)
> I have a version that behaves as proposed in comment #13.

Good.

> However there is one bad/surprising behaviour.
> If the current track in the playlist does not have a similar track and the
> bias looks for similar tracks, then the list just stays empty.
> 
> It's better for a rating:>5 search bias that clearly cannot be fulfilled,
> but it might be confusing for a "similar track" bias where you can't see why
> it's not working.
> 
> btw. if the current track has a similar track, then the list will be filled
> properly as the algorithm goes back and forth to find a row of tracks that
> all are similar.
> 
> Question: is that behaviour OK? Should the playlist accept "not so similar
> tracks" in some cases?

The behaviour is OK, in my opinion. Better provide nothing than actually lie to the user. (by adding a song that it not similar at all)

Note that it should be possible to work-around the starving by creating the following Dynamic Playlist:
* Match All Sequentially (ifElse bias)
*** Similar Track
*** Random Track

(^^^ Ralf, does this work as expected in your patch?) I would even go further and make the above the default "Play Similar Tracks" Dynamic playlist for new installations. (so that people could easily discover the trick)
Comment 17 Thomas Fjellstrom 2013-02-06 13:52:42 UTC
In the similar track case, would it be possible to match more than just the current track? Maybe a random sampling (searching the entire playlist would probably not be a good idea if the playlist is large). Just checking the current track leads to fairly large drift of what you're actually listening to over time I found in the past.
Comment 18 Matěj Laitl 2013-02-06 14:19:21 UTC
(In reply to comment #17)
> In the similar track case, would it be possible to match more than just the
> current track? Maybe a random sampling (searching the entire playlist would
> probably not be a good idea if the playlist is large). Just checking the
> current track leads to fairly large drift of what you're actually listening
> to over time I found in the past.

Perhaps, but in either case the discussion belong to a new feature request.
Comment 19 Ralf Engels 2013-02-07 15:25:55 UTC
Git commit 093177d4d8f86098b75d4a2ff28e23d4d0ceba6d by Ralf Engels.
Committed on 06/02/2013 at 18:38.
Pushed by rengels into branch 'master'.

Update Dynamic playlist. Get rid of energy optimizing code

The dynamic playlist always used to optimize the generated tracks
by different ways.
However the new biases all have hard conditions. The user also
used to see it like this and was confused if a "pretty good" result
was generated.

The new algorithm will generate an empty list if the conditions
cannot be fullfilled
Related: bug 292648, bug 304374, bug 301716
FIXED-IN: 2.8

M  +4    -0    ChangeLog
M  +10   -37   src/dynamic/Bias.cpp
M  +17   -31   src/dynamic/Bias.h
M  +46   -411  src/dynamic/BiasSolver.cpp
M  +21   -69   src/dynamic/BiasSolver.h
M  +6    -7    src/dynamic/BiasedPlaylist.cpp
M  +2    -1    src/dynamic/BiasedPlaylist.h
M  +7    -2    src/dynamic/TrackSet.cpp
M  +9    -7    src/dynamic/biases/AlbumPlayBias.cpp
M  +2    -2    src/dynamic/biases/AlbumPlayBias.h
M  +14   -15   src/dynamic/biases/EchoNestBias.cpp
M  +2    -3    src/dynamic/biases/EchoNestBias.h
M  +7    -11   src/dynamic/biases/IfElseBias.cpp
M  +3    -4    src/dynamic/biases/IfElseBias.h
M  +51   -39   src/dynamic/biases/PartBias.cpp
M  +3    -5    src/dynamic/biases/PartBias.h
M  +5    -4    src/dynamic/biases/QuizPlayBias.cpp
M  +2    -2    src/dynamic/biases/QuizPlayBias.h
M  +4    -5    src/dynamic/biases/TagMatchBias.cpp
M  +2    -2    src/dynamic/biases/TagMatchBias.h
M  +6    -6    src/services/lastfm/biases/LastFmBias.cpp
M  +2    -3    src/services/lastfm/biases/LastFmBias.h

http://commits.kde.org/amarok/093177d4d8f86098b75d4a2ff28e23d4d0ceba6d