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

Add conda-forge to Circle CI and include conda-forge installation instructions #521

Merged
merged 3 commits into from
Sep 8, 2020

Conversation

wd15
Copy link
Contributor

@wd15 wd15 commented Sep 3, 2020

No description provided.

@wd15 wd15 added this to the 0.4.1 milestone Sep 3, 2020
@wd15 wd15 self-assigned this Sep 3, 2020
@beyucel
Copy link
Contributor

beyucel commented Sep 4, 2020

I guess python setup.py install was the issue with the conda installation this solves #520

@beyucel
Copy link
Contributor

beyucel commented Sep 4, 2020

After checking with Linux and Windows, Linux works fine but windows installation still gives RandomizedPCA error.

Relax the testing standard for polynomial degree since all degrees
give similar results and the cross validation is
stochastic. Currently, it hasn't been determined how to make it
deterministic even with seeds.
@wd15
Copy link
Contributor Author

wd15 commented Sep 4, 2020

After checking with Linux and Windows, Linux works fine but windows installation still gives RandomizedPCA error.

You're still getting the same error from #520. I'll answer in that issue.

@wd15
Copy link
Contributor Author

wd15 commented Sep 5, 2020

@beyucel @auag92 @SvenPVoigt feel free to review this now

@beyucel
Copy link
Contributor

beyucel commented Sep 6, 2020

After checking with Linux and Windows, Linux works fine but windows installation still gives RandomizedPCA error.

You're still getting the same error from #520. I'll answer in that issue.

I just checked it again. It was a local problem. There is no problem with the Windows installation. I checked all notebooks (with conda-Windows).

@SvenPVoigt
Copy link
Contributor

SvenPVoigt commented Sep 8, 2020

Looks like the grid search parameter (in the fibers notebook) has some random variability to it. Is there any guarantee the check will pass with the new assertion 1<param<3? In general are there any guidelines on whether to check random variables or leave them out of tests?

@wd15
Copy link
Contributor Author

wd15 commented Sep 8, 2020

Looks like the grid search parameter (in the fibers notebook) has some random variability to it.

We don't want any variability ideally, but I haven't figured out how to seed it.

Is there any guarantee the check will pass with the new assertion 1<param<3?

Those are the only choices we're trying (from 1 to 3)! The poly degree doesn't have much influence.

In general are there any guidelines on whether to check random variables or leave them out of tests?

We shouldn't have randomness that isn't seeded as part of the tests. Just have to live with it for now unless we can figure out how to fix it.

@SvenPVoigt
Copy link
Contributor

Alright. Thanks for the answer!

Those are the only choices we're trying (from 1 to 3)! The poly degree doesn't have much influence.

Great, the test is guaranteed to pass then. I will approve.

@wd15
Copy link
Contributor Author

wd15 commented Sep 8, 2020

Alright. Thanks for the answer!

Those are the only choices we're trying (from 1 to 3)! The poly degree doesn't have much influence.

Great, the test is guaranteed to pass then. I will approve.

If you can figure out a way to make the example deterministic then I buy you a beer.

@wd15 wd15 merged commit eabc223 into materialsinnovation:master Sep 8, 2020
@wd15 wd15 deleted the conda-forge branch September 8, 2020 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants