Skip to content

GloVe#21

Merged
ynqa merged 8 commits intomasterfrom
glove
Dec 7, 2017
Merged

GloVe#21
ynqa merged 8 commits intomasterfrom
glove

Conversation

@ynqa
Copy link
Owner

@ynqa ynqa commented Oct 18, 2017

Overview

  • Implementation GloVe (on memory)
    • Solver is prepared SGD and AdaGrad.

Feature Works

  • GloVe with External Memory (write to disk once before training) like lexvec for huge data.

Repository owner deleted a comment from coveralls Oct 20, 2017
Copy link
Collaborator

@chewxy chewxy left a comment

Choose a reason for hiding this comment

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

left you some things to think about

P []tensor.Tensor
Q []tensor.Tensor

gradP []tensor.Tensor
Copy link
Collaborator

Choose a reason for hiding this comment

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

To think about: is it necessary to keep a copy of the tensors of the gradients?

Copy link
Owner Author

@ynqa ynqa Oct 21, 2017

Choose a reason for hiding this comment

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

Yes. GloVe uses AdaGrad as the optimizer, instead of SGD. AdaGrad also use all past iteration's gradients to decrease learning rate automatically.
Here: https://en.wikipedia.org/wiki/Stochastic_gradient_descent#AdaGrad

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I'm familiar with Adagrad. I'm thinking maybe this shouldn't be put into the Embedding struct. Instead have something like a Solver struct

Copy link
Owner Author

Choose a reason for hiding this comment

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

ah I see, that's right. I'll try it.
(It's also good to select other optimizers, AdaDelta, Adam, and so on.)


func (g *GloVe) train(pind, qind int, f float64) (err error) {
// SGD
inner, _ := tensor.Inner(g.emb.P[pind], g.emb.Q[qind])
Copy link
Collaborator

Choose a reason for hiding this comment

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

bad habit! should always check for errors.

Perhaps create a utility struct somewhere that looks like this:

type maybe struct{
    err error
}
func (m *maybe) DoBinOp(fn func(a, b interface{})(tensor.Tensor, error), a, b tensor.Tensor) (retVal tensor.Tensor) {
    if m.err != nil {
        return nil
    }
    retVal, m.err = fn(a, b)
    return
}

func (m *maybe) Error() error { return m.err }

then you can do this:

m := new(maybe)
inner := m.Do(tensor.Inner, g.emb.p[pind], g.emb.Q[qind])
bias := m.Do(tensor.Add, g.emb.biasP[pind, g.emb.biasQ[qind])
// and so on and so forth

Copy link
Owner Author

@ynqa ynqa Oct 21, 2017

Choose a reason for hiding this comment

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

Sure.

model/type.go Outdated
}

// OnesTensor create a tensor has 1 in all elements.
func (t *Type) OnesTensor(shape ...int) tensor.Tensor {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be called Ones... it already returns a Tensor, so no need to repeat

Copy link
Owner Author

@ynqa ynqa Oct 21, 2017

Choose a reason for hiding this comment

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

Sure.

@ynqa ynqa force-pushed the glove branch 2 times, most recently from be9fc29 to ad90f44 Compare October 28, 2017 06:40
Repository owner deleted a comment from coveralls Oct 28, 2017
Repository owner deleted a comment from coveralls Oct 28, 2017
Repository owner deleted a comment from coveralls Oct 28, 2017
@ynqa ynqa force-pushed the glove branch 2 times, most recently from abae4b5 to fd4d134 Compare October 30, 2017 08:38
Repository owner deleted a comment from coveralls Oct 30, 2017
Repository owner deleted a comment from coveralls Oct 30, 2017
Repository owner deleted a comment from coveralls Nov 1, 2017
Repository owner deleted a comment from coveralls Nov 1, 2017
@ynqa ynqa force-pushed the glove branch 2 times, most recently from 38e3416 to fb6a8a8 Compare November 1, 2017 17:36
Repository owner deleted a comment from coveralls Nov 6, 2017
@ynqa ynqa force-pushed the glove branch 2 times, most recently from 66d14b4 to f6192c6 Compare November 6, 2017 13:46
@coveralls
Copy link

coveralls commented Nov 6, 2017

Coverage Status

Coverage decreased (-10.0%) to 31.199% when pulling f6192c6 on glove into d310aec on master.

@coveralls
Copy link

coveralls commented Nov 7, 2017

Coverage Status

Coverage increased (+1.6%) to 42.793% when pulling 844fcd1 on glove into d310aec on master.

cost = 0.5 * fdiff * diff
fdiff *= a.initLearningRate

for i := 0; i < a.dimension; i++ {
Copy link
Collaborator

Choose a reason for hiding this comment

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

has there any work been done to compare this with a FMA function?

Copy link
Owner Author

@ynqa ynqa Nov 15, 2017

Choose a reason for hiding this comment

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

Done a little experiments: against about 30 million records
with FMA in tensor: 1 min per iteration
without FMA (this): 30 sec per iteration

// CofreqMap stores the co-frequency between word-word.
type CofreqMap map[Pair]float64

// Pair stores the co-frequency pair words.
Copy link
Collaborator

Choose a reason for hiding this comment

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

To make this even faster check this out: https://blog.chewxy.com/2017/07/12/21-bits-english/. I'll help you convert when I find the time

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'll watch it, thanks!

)

// GloVe stores the configs for GloVe models.
type GloVe struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

While GloVe is the proper name (Global Vector) I think it's quite terrible to have random Uppercase letters in the middle of a name that is not a word. Perhaps stick with Glove ? Just an opinion

Copy link
Owner Author

@ynqa ynqa Nov 15, 2017

Choose a reason for hiding this comment

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

Nope, so I'll rename it, by the way, why is it terrible there are uppercases in the middle?

@coveralls
Copy link

Coverage Status

Coverage increased (+1.8%) to 43.019% when pulling 5f85057 on glove into d310aec on master.

@coveralls
Copy link

coveralls commented Dec 3, 2017

Coverage Status

Coverage increased (+1.9%) to 43.062% when pulling 98ed1c7 on glove into d310aec on master.

@coveralls
Copy link

coveralls commented Dec 5, 2017

Coverage Status

Coverage increased (+1.8%) to 43.029% when pulling 3982f37 on glove into d310aec on master.

@ynqa ynqa merged commit 83aaee1 into master Dec 7, 2017
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.

3 participants