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

SpaceNet (this PR succeeds PR #219) #657

Merged
merged 836 commits into from Jul 28, 2015

Conversation

dohmatob
Copy link
Contributor

This PR succeeds PR #219 (aka unicorn factory). All discussions should be done here henceforth. #219 is now classified, and should be referred to solely for histological purposes.

@eickenberg
Copy link
Contributor

Wondering whether it may be useful to squash all commits before you start editing in there again. "Commits" tab is complaining about this PR being big ;)

Squashing should make it easier to keep this PR rebased while you finalize it.

Unless of course you are considering cherry-picking or removing some commits, but that doesn't seem necessary.

@dohmatob
Copy link
Contributor Author

On Tue, Jul 14, 2015 at 10:48 AM, eickenberg notifications@github.com
wrote:

Wondering whether it may be useful to squash all commits before you start
editing in there again.

No, I don't think so.

"Commits" tab is complaining about this PR being big ;)

Squashing should make it easier to keep this PR rebased while you finalize
it.

Unless of course you are considering cherry-picking or removing some
commits, but that doesn't seem necessary.


Reply to this email directly or view it on GitHub
#657 (comment).

DED

@eickenberg
Copy link
Contributor

As you wish.

It would have been a good opportunity to get rid of the merges from master that you did along the way, which make rebasing impossible. That was my main concern.
As a matter of fact, looking at the commit history, it may be even easier to kill the history altogether by copying and pasting the relevant files into a new PR.

@dohmatob
Copy link
Contributor Author

Good point. This is the workaround I was just about to adopt. The history is simply too big and too dirty. Squashing, rebasing, etc. here would take ages, and all this just to safe a few pages of more-harmful-than-useful code history. I'm copying the files to a fresh branch forked-off the upstream's master (nilearn/nilearn master), and then PR it. The old branch will still be available for histological purposes (if need be).

a principled way.
- Continuation is used along the regularization path, where the
solution of the optimization problem for a given value of the
regularization parameter `alpha` is used to used as initialization
Copy link
Member

Choose a reason for hiding this comment

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

is used as initialization

@dohmatob dohmatob force-pushed the sparse-models branch 2 times, most recently from cb39f7f to 245ccd8 Compare July 15, 2015 14:25
return energy, grad


def tv_l1_from_gradient(spatial_grad):
Copy link
Member

Choose a reason for hiding this comment

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

with a leading underscore (these are rather low-level functions, of which users should not be aware)

@AlexandreAbraham
Copy link
Contributor

As agreed, I am going to merge this PR now. I lock it during the process.

@nilearn nilearn locked and limited conversation to collaborators Jul 28, 2015
@AlexandreAbraham
Copy link
Contributor

I created micro issues for the remaining comments.

For the future: please try to break this kind of PR into smaller ones.

For now: Great job, let's celebrate before being invaded by bugs 👯

Clicking the button!

@nilearn nilearn unlocked this conversation Jul 28, 2015
AlexandreAbraham added a commit that referenced this pull request Jul 28, 2015
@AlexandreAbraham AlexandreAbraham merged commit 32c1b33 into nilearn:master Jul 28, 2015
@GaelVaroquaux
Copy link
Member

Champagne! 🍷

@dohmatob
Copy link
Contributor Author

gr8! ✌️ 🍷 🍷 🍷

@dohmatob
Copy link
Contributor Author

On Wed, Jul 29, 2015 at 12:28 AM, Alexandre Abraham <
notifications@github.com> wrote:

I created micro issues for the remaining comments.

For the future: please try to break this kind of PR into smaller ones.

Yes. mosdef.

For now: Great job, let's celebrate before being invaded by bugs [image:
👯]

Clicking the button!


Reply to this email directly or view it on GitHub
#657 (comment).

DED

@agramfort
Copy link
Contributor

agramfort commented Jul 29, 2015 via email

@dohmatob
Copy link
Contributor Author

Yeah man :)

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

10 participants