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

Fix the behaviour of reverse that messes up the critical diagrams #11

Closed

Conversation

AlexandreAbraham
Copy link

I have observed on my machine the same behaviour as in issue #10 . It happens even when plotting the example. However, setting reverse=False manually in the code yielded the correct behaviour. I managed to find conditions where it works for reverse and not reverse. Note that in a venv with downgraded version of a lot of package (I did not investigate further), I have the correct behaviour without the fix, so actually I do not know the origin of the problem.

If somebody more knowledgeable than me could have a deeper look, that would be great!

@sherbold
Copy link
Owner

sherbold commented Jun 4, 2022

@AlexandreAbraham Thanks for the PR! I looked into this, but the problem seems to run deeper. I found a test case, which fails to generate the correct plots for either ascending or descending order. Your fix just changes which order fails.

    def test_brokenplots(self):
        stds = [0.1, 0.1, 0.5, 0.1, 0.05, 0.05]
        means = [0.2, 0.3, 0.5, 0.8, 0.85, 0.9]
        data = pd.DataFrame()
        for i, mean in enumerate(means):
            data['pop_%i' % i] = np.random.normal(mean, stds[i], self.sample_size).clip(0, 1)
        res_asc = autorank(data, 0.05, self.verbose, order='ascending')
        res = autorank(data, 0.05, self.verbose)
        # currently one plot is always broken (on my machine)
        plot_stats(res)
        plot_stats(res_asc)
        plt.draw()
        plt.show()

I am currently using the following library versions locally (did not update in some time):

baycomp==1.0.2
cycler==0.11.0
fonttools==4.28.5
kiwisolver==1.3.2
matplotlib==3.5.1
numpy==1.21.5
packaging==21.3
pandas==1.3.5
patsy==0.5.2
Pillow==8.4.0
pyparsing==3.0.6
python-dateutil==2.8.2
pytz==2021.3
scipy==1.7.3
six==1.16.0
statsmodels==0.13.1

I am not sure what exactly the issue is yet. If you find the time, could you check if the test works for you correctly and, if it works, check the versions of the dependencies you are using?

@AlexandreAbraham
Copy link
Author

@sherbold Indeed in your test I see one broken plot. Let me maybe give you more details on what I observed:

  • I ran autorank and realized that the CD plot was broken
  • When looking at the actual data, I realized that top5, which was supposed to be on the right, was on the left, hence the weird lines.
  • I have manually forced reverse=True in the calling to the plotting function and observed that the original code was working in that setting.
  • I understood that part of the code was expecting the data to be sorted in a certain way and fixed this behavior. I guess that the descending / ascending also reverses the order and constitute a new edge case.

My best guess is that one should not expect the data to be sorted in a certain way for the display. This would make the code more reliable.

@sherbold sherbold mentioned this pull request Jul 2, 2022
@sherbold
Copy link
Owner

This is now (hopefully) solved with commit d7df9d9. If the issue still exists, please open a new issue.

@sherbold sherbold closed this Mar 15, 2023
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.

None yet

2 participants