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

Added ranking by elimination test #110

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

kishorkl
Copy link

Added a ranking by elimination test. This is a multi-stimulus test without the need for a reference. The test is similar to paired comparison but much faster to perform when there are many conditions. This test has been compared to paired comparison test in https://www.aes.org/e-lib/browse.cfm?elib=14979

@faroit
Copy link
Collaborator

faroit commented Apr 16, 2023

Wow. This is incredible useful. I will look at the paper next week and test the code.

Copy link
Collaborator

@faroit faroit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good so far but i have some question for the method.

And it would be nice if you can share the paper with me as I don't have access to AES.

lib/webmushra/pages/RankingPage.js Show resolved Hide resolved
lib/webmushra/pages/RankingPage.js Outdated Show resolved Hide resolved
lib/webmushra/pages/RankingPage.js Outdated Show resolved Hide resolved
lib/webmushra/pages/RankingPage.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@faroit faroit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing the code, works nicely. But for some applications it would still be nice to be able to have a reference to be played (which could be optional of course). This has applications e.g. for the waveform renderering as pointed out in the comment on the review.

@kishorkl how does literature comment on references in ranking based tests?

this.macic = new MushraAudioControlInputController(this.mushraAudioControl, this.pageConfig.enableLooping);
this.macic.bind();

this.waveformVisualizer = new WaveformVisualizer(this.pageManager.getPageVariableName(this) + ".waveformVisualizer", tdRight, this.conditions[0], this.pageConfig.showWaveform, this.pageConfig.enableLooping, this.mushraAudioControl);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't it an arbitrary decision to select this.conditions[0] as the condition that is used to render the waveform? I feel like overall it would be better to use the reference for this and therefore give experiments control over which condition or even the reference is used?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kishorkl can you comment on this one please?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kishorkl do you have time to finish this PR, I would like to issue a new version soon

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@faroit you are right that using this.conditions[0] to plot the waveforms is arbitrary. I would need to reread the paper to check what they suggest in terms of using the reference. I will get back on this in two weeks.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. Thanks

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original paper basically suggests ranking by elimination as an alternative to a paired comparison test - which compares two models, without one marked as a reference or groundtruth (So AB test and not an ABX test). They don't compare to MUSHRA like tests. So there is no mention of the "reference" condition. Since we are looking at an optional feature, I will implement a "reference", which would be used for the waveforms if present (and also with a new optional button). I am still unsure what could be used for the waveforms when the "reference" is absent.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi, do you have some update on this one?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kishorkl do you have time to finish this?

@kishorkl
Copy link
Author

kishorkl commented Jul 19, 2023

looks good so far but i have some question for the method.

* How does this compare to [Paired comparison without a reference #88](https://github.com/audiolabs/webMUSHRA/issues/88) ?

This is almost entirely different from the mentioned commit. In the current submission one can rank multiple items, whereas in the referred commit, preference between "only" two items is tested. The current submission can find a preference between two items as well.

* Is it part of the test design that a condition has to be removed instead of reordering or actually ranking them?

And it would be nice if you can share the paper with me as I don't have access to AES.

The paper is available at https://www.mathpsy.uni-tuebingen.de/wickelmaier/pubs/WickelmaierUmbach2009AES.pdf

kishorkl and others added 5 commits July 21, 2023 15:00
Bumps [qs](https://github.com/ljharb/qs) from 6.5.2 to 6.5.3.
- [Release notes](https://github.com/ljharb/qs/releases)
- [Changelog](https://github.com/ljharb/qs/blob/main/CHANGELOG.md)
- [Commits](ljharb/qs@v6.5.2...v6.5.3)

---
updated-dependencies:
- dependency-name: qs
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [braces](https://github.com/micromatch/braces) from 3.0.2 to 3.0.3.
- [Changelog](https://github.com/micromatch/braces/blob/master/CHANGELOG.md)
- [Commits](micromatch/braces@3.0.2...3.0.3)

---
updated-dependencies:
- dependency-name: braces
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@faroit
Copy link
Collaborator

faroit commented Sep 17, 2024

@kishorkl I would love to merge this, so maybe we can shortcut this and decide on:

  • do not render the waveform at all to avoid the confusion which waveform should be used as we don't have reference
  • do not do a reference.

This should be fairly easy to do, right?

@kishorkl
Copy link
Author

@kishorkl I would love to merge this, so maybe we can shortcut this and decide on:

* do not render the waveform at all to avoid the confusion which waveform should be used as we don't have reference

* do not do a reference.

This should be fairly easy to do, right?

Yes. I got occupied with some other tasks, hence could not spend much time, sorry. I will try to implement this during this week.

@kishorkl
Copy link
Author

kishorkl commented Sep 20, 2024

@faroit Now the changes we discussed should be done. The ranking test does not have the ability to show a waveform and does not have reference. I also did a bug fix which was found in saving the results of the ranking test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants