Skip to content
This repository was archived by the owner on Nov 23, 2018. It is now read-only.

Conversation

btracey
Copy link
Member

@btracey btracey commented Jun 11, 2016

No description provided.

@btracey
Copy link
Member Author

btracey commented Jun 11, 2016

PTAL @sbinet

extractColumns(ab, A, newBasic)
xb, err := initializeFromBasic(ab, b)
newBasic[addedIdx] = i
if set {
Copy link
Member

Choose a reason for hiding this comment

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

this is probably just very nit-picking but I would probably reverse the if-else-branch:

if !set {
   set = true
   extractColumns(ab, A, newBasic)
} else {
   mat64.Col(col, i, A)
   ab.SetCol(addedIdx, col)
}

that way, the test on set and its first (and last) modification are closer, visually.

(when I read it first, I was a bit confused whether this wasn't just dead code...)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I accidentally nuked my local version of this... I'll submit this in the next PR

@sbinet
Copy link
Member

sbinet commented Jun 13, 2016

just a little nitpick, but LGTM otherwise.

@btracey btracey merged commit 802f208 into master Jun 13, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants