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

WIP: linalg cleanups #2069

Closed
wants to merge 5 commits into from
Closed

WIP: linalg cleanups #2069

wants to merge 5 commits into from

Conversation

ViralBShah
Copy link
Member

This branch will track all the linalg work related to #2062.

(:zgeev_,:zgesvd_,:zgesdd_,:Complex128),
(:cgeev_,:cgesvd_,:cgesdd_,:Complex64))
for (geev, gesvd, gesdd, elty, relty) in
((:dgeev_,:dgesvd_,:dgesdd_,:Float64,Float64),
Copy link
Member

Choose a reason for hiding this comment

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

One more...

dmbates and others added 3 commits January 18, 2013 09:29
Add gemm and gemv methods that have an implicit 1.0 multiplier
Conflicts:
	test/linalg.jl
@andreasnoack
Copy link
Member

I have changed dot to use BLAS for complex types and also added some convenience methods to Blas.jl. Now I want to make a pull request to this pull request but I have rebased master upon my branch and hence my pull request consist of 99 commits in 98 files or something like that. How should I continue?

@ViralBShah
Copy link
Member Author

Since you have commit access to juliaLang, you should be able to checkout vs/linalg and directly push to this branch.

I almost did the same thing last night. :-)

@andreasnoack
Copy link
Member

The thing that I am unsure about is if I should push with my rebased version or if it would cause problems. If I just change dot on vs/linalg as it is now, it wouldn't work as the branch is behing master. Does my confusion make sense?

@ViralBShah
Copy link
Member Author

I do not know enough about git to say what would happen if you push your rebased version. Perhaps @pao or @StefanKarpinski can suggest.

@StefanKarpinski
Copy link
Member

It might be easier to just take the diff between master and your branch and just making a patch from that. For example, on master you can just do something like this:

git diff master branchname | git apply

Now you'll have all this changes applied to master but not checked in. You can commit than and then scrap the messed up branch. You may want to make sure that the first diff argument is chosen so that only the relevant changes are included.

@andreasnoack
Copy link
Member

It doesn't work for me. Is there a reason not to rebase master onto vs/linalg. I am just trying to understand collaborative git here. If it is a bad idea I'll just separate out the complex dot thing and make a request of that directly on master.

@StefanKarpinski
Copy link
Member

This might just be a terminology thing, but rebasing master onto vs/linalg would be rather bad – that means replaying the commits that have happened in master since it diverged from vs/linalg on top of the current state of vs/linalg and then making the result of that replay the new master. Here's a diagram:

before:

    A -- B -- C -- D <-- master
     \
      E -- F -- G -- H <-- vs/linalg

after rebase of master onto vs/linalg

    A -- B -- C -- D
     \
      E -- F -- G -- H <-- vs/linalg
                      \
                       B' -- C' -- D' <-- master 

From this picture, you can probably see that this is not what you want. Rewriting history on master is a general no-no, and you can only push the fabricated history with a git push -f to master. I wish there was some way to disable doing that, but as far as I know there isn't. If someone who already has master up to D pulls after you've done this, they will end up doing a merge of their master – i.e. D – with the new master D', resulting in this history:

after merge of old master and new master:

    A -- B -- C -- D -------------------- M <-- master
     \                                  /
      E -- F -- G -- H -- B' -- C' -- D'

Clearly, that's a very messed up history. The other direction – rebasing vs/lingalg onto master might be what you mean. That would look like this:

after rebase of vs/linalg onto master:

    A -- B -- C -- D <-- master
                    \
                     E' -- F' -- G' -- H' <-- vs/linalg

That may also be problematic under similar circumstances, however – if someone else, like Viral, who owns the branch by convention because of the vs/ prefix, tries to do a merge while still having the old version H of vs/linalg. In that case you'd end up with this:

after merge of old vs/linalg and new vs/linalg:

    A -- B -- C -- D <-- master
     \              \
      \              E' -- F' -- G' -- H'
       \                                \
        E -- F -- G -- H --------------- M' <-- vs/linalg

Since this history isn't in master immediately, it's not so awful, but it's still obviously kind of a bad history. So if you're going to rebase some branch that someone else might have a copy of, you want to be sure they aren't going to try to pull from it and then push the resulting merge later. Git is unfortunately not very helpful at preventing these things or facilitating that communication (disallowing forced pushes would be a good way to prevent this), but that's how it is.

The sanest thing to do here is probably to make your own version of Viral's branch and call it anj/linalg. Then you can do git rebase master to replay all of the commits on vs/linalg on top of master. You can push that new fabricated history to your own branch without danger of anyone pulling it and creating a nasty merge commit of both versions of the history.

@andreasnoack
Copy link
Member

Great. Thank you for the detailed explanation. I interchanged the rebasing so I meant the latter of your two examples, but anyway I see the problem. I'll split up my changes.

@pao
Copy link
Member

pao commented Jan 27, 2013

To summarize Stefan's detailed explanation, you don't want to rebase master; you also don't want to rebase vs/linalg because you and Viral are sharing it. You should only rebase (which changes history) on branches that are yours and yours alone.

@ViralBShah
Copy link
Member Author

I am closing this pull request and creating a new one.

@ViralBShah ViralBShah closed this Feb 4, 2013
@ViralBShah ViralBShah deleted the vs/linalg branch February 4, 2013 01:18
ViralBShah added a commit that referenced this pull request Feb 4, 2013
Relevant comments are in #2062.
ViralBShah added a commit that referenced this pull request Feb 9, 2013
  commit 5c1e646
    Chain methods for gemm and gemv to gemm! and gemv!
    Add gemm and gemv methods that have an implicit 1.0 multiplier
  commit 11c8f36
    Chain more foo methods to foo!, add blas tests.
    Reformat the linalg tests.

Relevant comments are in #2062.

Conflicts:
	test/Makefile
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.

6 participants