Skip to content

Implemented 'tree' and 'full' edge structure flags in MultiLabelClf#174

Open
almath123 wants to merge 3 commits into
pystruct:masterfrom
almath123:new_edge_structures_mlc
Open

Implemented 'tree' and 'full' edge structure flags in MultiLabelClf#174
almath123 wants to merge 3 commits into
pystruct:masterfrom
almath123:new_edge_structures_mlc

Conversation

@almath123
Copy link
Copy Markdown

I have implemented the "tree" and "full" edge structures that were mentioned in the doc comments for MultiLabelClf but not previously implemented. Most of the hard work was already done in the multi_label example, I just moved it into the library and added some test cases.

One thing that I have done that is perhaps a bit messy is to divide the code to set the edge structure into two methods one which does initialization when no labels are needed (edges is None or 'full') and another that needs the labels (edges is 'tree'). I did this because the existing multi label test cases run inference on the model without initializing. I'm not convinced this is the right approach.

Thoughts? Comments?

@amueller
Copy link
Copy Markdown
Member

amueller commented Feb 8, 2016

Thanks for working on this! I think we need to increase the scipy version requirement for the spanning tree function. Hm it looks like that is not actually documented (my bad), but the continuous integration runs with scipy==0.9.0, which doesn't have the function. Can you check when it was introduced and change the .travis.yml so the tests run through?

@amueller
Copy link
Copy Markdown
Member

amueller commented Feb 8, 2016

Fixes #88.

Comment thread pystruct/models/multilabel_svm.py Outdated
if self.n_features is not None and self.n_states is not None:
if self.edges is None:
self.edges = np.zeros(shape=(0, 2), dtype=np.int)
self._set_edge_structure()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is not necessary. initialize should have been called before _set_size_joint_feature If initialize has not been called, it's very likely that self.n_features and self.n_states is unknown yet.

@almath123
Copy link
Copy Markdown
Author

Thanks for the comments.

I have setup scipy to be installed via pip. It takes a long time to install, but passes all the tests in the two "ubuntu" environments now. I haven't been able to get it to work in the "conda" environments yet, it doesn't seem to be able to find the mkl libraries. I'm not even sure why it needs mkl.

I also modified the code, as per your comment, so that initialize is the only method to call _set_edge_structure_from_labels which means that calling initialize is required to setup the edge structure (if None, "full" or "tree" are passed).

@amueller
Copy link
Copy Markdown
Member

ok this sounds great. I'm super busy right now but I'll try to have a look at the travis failures sometime soon.

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.

2 participants