Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

learning FactorGraphModel #1538

Merged
merged 1 commit into from Sep 9, 2013
Merged

Conversation

hushell
Copy link
Contributor

@hushell hushell commented Sep 6, 2013

I know this is a huge PR again. Sorry about this. I added 2 classes: FactorGraphModel and FactorGraphFeatures. I tested PrimalMosek solver and BMRM solver, at least the simple example works.

@ppletscher, @iglesias Please help me check the code. For the loss-augmentation, I am not sure it should be in MAPInference or in the model class, it's related to inference, but basically it modifies the energy table.

@iglesias
Copy link
Collaborator

iglesias commented Sep 6, 2013

Hi @hushell! What about starting off by separating the CMake part from the rest? I think that each of them is worth a separate pull request and also their own commits.

@hushell
Copy link
Contributor Author

hushell commented Sep 6, 2013

Sounds great! So I send 2 PRs at the same time?

@iglesias
Copy link
Collaborator

iglesias commented Sep 6, 2013

Yep, you just need to submit them from different branches.

@hushell
Copy link
Contributor Author

hushell commented Sep 6, 2013

updated! :)

w[5] = 0.6; // 0,1
w[6] = -0.2; // 1,1
w[7] = 0.75; // 1,1
//float64_t w_norm = SGVector< float64_t >::twonorm(w.vector, w.vlen);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove commented code that is not used please.

@iglesias
Copy link
Collaborator

iglesias commented Sep 6, 2013

I've got a doubt regarding the labels and features of the FactorGraphMdoel. The labels used for the FactorGraphModel are structured labels where each of them is a FactorGraph, right? Also, the features are a set of FactorGraphs as well?

@iglesias
Copy link
Collaborator

iglesias commented Sep 6, 2013

Could you also maybe paste in here the class diagram that you did for the google doc at the beginning? I think there have been some updates. I find it difficult to keep track of what each class should be doing :)

@hushell
Copy link
Contributor Author

hushell commented Sep 6, 2013

Sure. I'll draw a new diagram. Now I understand more, and many things have been updated. Let me briefly explain what's going on with FactorGraphFeatures and FactorGraphLabels and FactorGraph.

First of all, a sample of FactorGraph is a structured feature, which should be stored in the FactorGraphFeatures (has an array m_samples). By concatenating data in FactorGraph, we get psi, the joint feature map. Here we assume that the FactorGraphModel has a linear parameterization.

Then, the FactorGraphLabels store an array of structured outputs, i.e. m_labels. We defined a class FactorGraphObservation for representing the structured output. In our case, a structured output is vector of states (each node in factor graph has a discrete state). So in fact, no structure encoded in the FactorGraphObservation, all structured information are recorded in FactorGraph.

@iglesias
Copy link
Collaborator

iglesias commented Sep 6, 2013

Aham! I understand more about it now. Thank you so much for the detailed explanation!

@hushell
Copy link
Contributor Author

hushell commented Sep 7, 2013

Updated! However Travis was unhappy about something.

@iglesias
Copy link
Collaborator

iglesias commented Sep 7, 2013

In the current develop I think the RubyModular issue in Travis has already been fixed, so just rebase this branch and force push.

@@ -67,7 +67,7 @@ bool CPrimalMosekSOSVM::train_machine(CFeatures* data)
// Number of auxiliary constraints
int32_t num_aux_con = m_model->get_num_aux_con();
// Number of training examples
int32_t N = m_model->get_features()->get_num_vectors();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mmm I wonder why this was not causing a memory leak before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess previous models doesn't hold a DynamicObjectArray for features.

Copy link
Collaborator

Choose a reason for hiding this comment

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

CStructuredModel::get_features() does

SG_REF(m_features)

How is it related to DynamicObjectArray?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, m_features is a DynamicObjectArray... m_model->get_features() is my old change from your code, I remember there was a get_features() in StructuredOutputMachine at beginning. But no idea, why no meak leak happens.

Copy link
Collaborator

Choose a reason for hiding this comment

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

m_features is CFeatures* I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then, I see the point, for multiclass and hmsvm, features are SGVector or SGMatrix, but in my case, m_features is FactorGraphFeatures, where I hold a DynamicObjectArray for storing FactorGraphs.

iglesias added a commit that referenced this pull request Sep 9, 2013
learning FactorGraphModel
@iglesias iglesias merged commit ec619b3 into shogun-toolbox:develop Sep 9, 2013
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 4244455 on hushell:develop into * on shogun-toolbox:develop*.

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.

None yet

3 participants