pystruct / pystruct

Simple structured learning framework for python
http://pystruct.github.io
BSD 2-Clause "Simplified" License
665 stars 174 forks source link

wrong `n_latent` in `EdgeFeatureLatentNodeCRF` with `latent_node_features=True` #223

Closed WladimirSidorenko closed 6 years ago

WladimirSidorenko commented 6 years ago

Description: When using EdgeFeatureLatentNodeCRF with activated latent_node_features, the number of hidden states is determined incorrectly while computing _get_unary_potentials()

How to reproduce:

from pystruct.learners import FrankWolfeSSVM
from pystruct.models import EdgeFeatureLatentNodeCRF
import numpy as np

model = EdgeFeatureLatentNodeCRF(n_labels=3, n_features=3, n_edge_features=2, n_hidden_states=3, latent_node_features=True)
model = FrankWolfeSSVM(model)

node_feats = np.random.rand(5, 3)
edges = np.concatenate((np.expand_dims(np.arange(0, 4), -1), np.expand_dims(np.arange(1, 5), -1)), axis=1)
edge_feats = np.random.rand(edges.shape[0], 2)
n_hidden = 4
y = np.random.randint(0, 3, 5)

model.fit([(node_feats, edges, edge_feats, n_hidden)], [y])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/sidorenko/Projects/DASA/venv/lib/python3.5/site-packages/pystruct/learners/frankwolfe_ssvm.py", line 299, in fit
    self._frank_wolfe_bc(X, Y)
  File "/home/sidorenko/Projects/DASA/venv/lib/python3.5/site-packages/pystruct/learners/frankwolfe_ssvm.py", line 222, in _frank_wolfe_bc
    y_hat, delta_joint_feature, slack, loss = find_constraint(self.model, x, y, w)
  File "/home/sidorenko/Projects/DASA/venv/lib/python3.5/site-packages/pystruct/utils/inference.py", line 65, in find_constraint
    y_hat = model.loss_augmented_inference(x, y, w, relaxed=relaxed)
  File "/home/sidorenko/Projects/DASA/venv/lib/python3.5/site-packages/pystruct/models/latent_node_crf.py", line 526, in loss_augmented_inference
    unary_potentials = self._get_unary_potentials(x, w)
  File "/home/sidorenko/Projects/DASA/venv/lib/python3.5/site-packages/pystruct/models/latent_node_crf.py", line 517, in _get_unary_potentials
    unaries[:n_visible, self.n_labels:] = -1e2 * max_entry
TypeError: only integer scalar arrays can be converted to a scalar index

How to solve: Line 496 in file latent_node_crf.py uses x[2] to determine the number of hidden states in the input, but according to the documentation of the EdgeFeatureLatentNodeCRF class, x[2] should be an array of edge features. So changing

            n_hidden = x[2]

to

            n_hidden = x[-1]

should solve the problem.

Environment:

>uname -srm
Linux 4.4.0-53-generic x86_64

>python --version
Python 3.5.2

>pip show pystruct
Name: pystruct
Version: 0.2.4
jlmeunier commented 6 years ago

Hi Wladimir, thanks for this post. I've include your fix in the code.

WladimirSidorenko commented 6 years ago

Hi Jean-Luc,

Thanks a lot for the fix.

Fixed in 83f6b6963fd41c0026ba5ae9d3c55b972fa0832f