diprism / fggs

Factor Graph Grammars in Python
MIT License
13 stars 3 forks source link

sum_product.py: options -g (gradient) -e (expect) -n (normalize) #149

Closed ccshan closed 2 years ago

davidweichiang commented 2 years ago

Isn't dim=0 arbitrary?

davidweichiang commented 2 years ago

Could you explain what the problem was without requiresgrad?

ccshan commented 2 years ago

Could you explain what the problem was without requiresgrad?

Just that no gradient gets computed and so weights.grad throws an error. If you don't see that then we have some sort of version mismatch.

Isn't dim=0 arbitrary?

Perhaps but the default dim=1 doesn't work for a 1D tensor like unfair=[0.6,0.4]

davidweichiang commented 2 years ago

I cherry-picked your requires_grad fix into #152 but I'm not sold on a -n option that has dim=0 hard-coded into it...

ccshan commented 2 years ago

Ok, does it make more sense to hard-code dim=ndim-1 ?

davidweichiang commented 2 years ago

I'm wary of any kind of hard-coding at all. Is this so that the gradient of the von Neumann example will work out to zero?

davidweichiang commented 2 years ago

How about making the command-line option -n factor dim?

ccshan commented 2 years ago

I'm wary of any kind of hard-coding at all. Is this so that the gradient of the von Neumann example will work out to zero?

Yes (or rather 1e-8).

How about making the command-line option -n factor dim?

Ok.