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

Rand 0.6 -> 0.7 #660

Merged
merged 2 commits into from
Oct 15, 2019
Merged

Rand 0.6 -> 0.7 #660

merged 2 commits into from
Oct 15, 2019

Conversation

dodomorandi
Copy link
Contributor

@dodomorandi dodomorandi commented Oct 8, 2019

With the new version of rand it is possible to use Distribution<f32> with Matrix::from_distribution, which is pretty handy.

  • Bumped rand version to 0.7
  • Added dependency to rand_distr
  • Bumped quickcheck version to 0.9 (because of rand)
  • Bumped rand_xorshift version to 0.2

@dodomorandi
Copy link
Contributor Author

I think that this is pretty redudant with #622, but at the same time it includes only the dependency update, without any particular tweaks.

 * Bumped rand version to 0.7
 * Added dependency to rand_distr
 * Bumped quickcheck version to 0.9 (because of rand)
 * Bumped rand_xorshift version to 0.2
@dodomorandi
Copy link
Contributor Author

It looks like there is some sort of numeric instability of worse, because I was not able to reproduce the failing tests shown by travis (different between nightly and beta), and after some tests I decided to re-run the CI on my repo... and all tests are passing.

The issue does not seem to be related to my PR, but I think that it is interesting and it should be taken into account as a possible problem.

@sebcrozet
Copy link
Member

Thank you for this PR @dodomorandi! Yes, I think it's a good idea to have a PR dedicated to the rand update. The tweaks are actually why #622 has not been merged yet.

You are right, it can happen that the test fail sometimes if the randomly generated input matrices were badly conditioned. We've not taken the time to address this, so I usually just restart the CI and see if it keeps failing (in which case the problem would likely be caused by the PR).

@sebcrozet sebcrozet merged commit afab848 into dimforge:dev Oct 15, 2019
@sebcrozet
Copy link
Member

sebcrozet commented Oct 15, 2019

The CI passes now, thanks!

@sebcrozet sebcrozet added breaking change Fixing this issue, or merging this pull-request is likely to require breaking changes enhancement P-medium Medium priority labels Oct 15, 2019
@cuviper cuviper mentioned this pull request Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Fixing this issue, or merging this pull-request is likely to require breaking changes enhancement P-medium Medium priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants