search

cognoma / machine-learning

Machine learning for Project Cognoma
Other
32 stars 47 forks source link

Dask search cv #98

Closed rdvelazquez closed 7 years ago

rdvelazquez commented 7 years ago

This notebook shows the implementation of dask-searchcv to speed up the pipeline and allow for implementing PCA in cross-validation. This PR is the first step in addressing Issue #94 ... The next step will be to implement dask-searchcv in the main notebooks after #93 is merged.

@dhimmel: I'm not sure why this is showing 10 commits (it's including the commits from PR #95). This may have something to do with the fact that I started #95 by just uploading files on github.com and/or that I was accidentally using my master branch. Let me know if you have any advice or input on that. Sorry about my lack of git experience (I think I'm finally getting it now).

dhimmel commented 7 years ago

I'm not sure why this is showing 10 commits

Can you rebase on the current version of cognoma:master?

git fetch upstream
git rebase upstream/master

This will replay your commits on top of the latest cognoma commits and remove the commits that are essentially duplicates.

Then you will have to git push --force.

rdvelazquez commented 7 years ago

@dhimmel Thank you for the pointers! What you recommended makes sense but I'm still having some trouble rebasing. I suspect the issues are associated with the two errors I made with my original pca pull request: (1) editing and pull requesting from my master branch and (2) deleting and re-uploading files with the same filenames.

Would starting from scratch be an option? i.e. deleting my forked and local repos, re-forking and cloning the cognoma/machine-learning repo and starting a new (correct) pull request for this daskSearchCV notebook?

If that isn't an option and you'd prefer me to rebase, I'll try to figure out the issues I'm having and I can post my error messages/get some help from you if I can't figure out how to solve the issues with online resources.

dhimmel commented 7 years ago

Would starting from scratch be an option?

Perhaps, but maybe a bit extreme. I think you should be able to fix it on a branch by branch basis.

On your master branch you can do:

git fetch upstream
git reset --hard upstream/master

This should make your master branch equal cognoma:master (as we want).

I'm still having some trouble rebasing.

What issues are you having? Since you're rebasing on upstream/master, your local master branch should not matter for this operation.

rdvelazquez commented 7 years ago

So I reset my master branch; it's now even with cognoma:master. However when I reset my dask-searchCV branch it not only removed the duplicate commits but also the commits where I uploaded the dask-searchCV notebook (so the dask-searchCV branch was then also even with cognoma:master), which seems to have closed this pull request.

I re-added the dask-searchCV notebook (and .py file) to my dask-searchCV branch, thinking it might automatically re-open this pull request but it didn't. I think if I pull request the dask-searchCV branch again it may open up a new pull request and I think we want to keep this under this PR (#98).

Any advice on how to proceed. Thank you so much for the help and for working with me!

What issues are you having? Since you're rebasing on upstream/master, your local master branch should not matter for this operation.

Based on my limited knowledge of git and what I was looking up online, I was getting merge conflicts for duplicate files. The advice I found online (stack overflow and the like) was to use git checkout --theirs (filename) to deal with the merge conflict (essentially telling git to just use the cognoma:master file in cases of conflict but that didn't seem to completely remedy the situation. I don't still have the error messages from those attempts (I wish I had saved them so I could post for your reference).

dhimmel commented 7 years ago

I think if I pull request the dask-searchCV branch again it may open up a new pull request and I think we want to keep this under this PR (#98).

Most likely. Don't worry about opening a new PR. Just in that PR, mention that it continues on https://github.com/cognoma/machine-learning/pull/98.

rdvelazquez commented 7 years ago

Will do. Thanks!

Remember that time I said "Sorry about my lack of git experience (I think I'm finally getting it now)." and then proceeded to basically break git... :open_mouth: