WIP Write Generalized Linear Machine class

[Hephaestus12]

#5005 #5000
This is the basic framework for the Generalized Linear Machine class.
This class is supposed to implement the following distributions
BINOMIAL,
GAMMA,
SOFTPLUS,
PROBIT,
POISSON

The code has been written keeping in mind this reference: PyGLMNet library
However, I have only written code for the Poisson distribution till now.

THIS IS A WORK IN PROGRESS

This PR is so that a discussion can be held about the implementation of the GLM and so that Some feedback can be obtained for my code.
@lgoetz
@geektoni

TODO

• Write code.
• Add basic test.
• Add gradient test.
• Link github gists for generating data.
• Check why the SGObject test is failing.
• Use FeatureDispatchCRTP.

[vigsterkr]

great stuff i've added some comments... until this is WIP could you make this PR a draft actually?

[Hephaestus12]

Everything added and the unit test works fine, the gist added for the first unit test.

However, the meta example test fails. Output: https://pastebin.com/fw5aS77U

[geektoni]

Everything added and the unit test works fine, the gist added for the first unit test.

However, the meta example test fails. Output: https://pastebin.com/fw5aS77U

It seems that when creating the labels in the meta example it treats them as MulticlassLabels rather than RegressionLabels. Is there any way to force the label's creation to adopt a certain type of features? @gf712 @karlnapf

You have also a conflict with the data submodule. I think you need to rebase against the develop branch.

[geektoni]

We are getting there! :) once the meta example is fixed, you should also add the meta example serialization result to the data submodule such to enable the integration tests.

[gf712]

Everything added and the unit test works fine, the gist added for the first unit test.
However, the meta example test fails. Output: https://pastebin.com/fw5aS77U

It seems that when creating the labels in the meta example it treats them as MulticlassLabels rather than RegressionLabels. Is there any way to force the label's creation to adopt a certain type of features? @gf712 @karlnapf

You have also a conflict with the data submodule. I think you need to rebase against the develop branch.

I think there are two options: wait for the work @LiuYuHui is doing right now implementing the label encoding at Machine level, or you can use the regression_labels free function in the GLM train and apply routines to convert any labels to RegressionLabels if possible.

[karlnapf]

I think use regression_labels for now, and as this should be done before the other refactor.

[geektoni]

I think regression_labels cannot be used, since the error in the gist seems to be thrown by regression_labels itself.

…

On Thu, 30 Jul 2020 at 11:57, Heiko Strathmann ***@***.***> wrote: I think use regression_labels for now, and as this should be done before the other refactor. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5006 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZE2KEB3KOHOFMEKMSGSWLR6E7XVANCNFSM4MHT627Q> .

[geektoni]

So basically it is not possible to manually create RegressionLabels without calling regression_labels, right?

If that is the case, @Hephaestus12 I think we should remove the glm meta example from here and open a new PR with it. That way we could merge directly this GLM code and then add the meta example later on (since the issue with it is not with the GLM itself).

[gf712]

Ah yes, it isn't implemented, even though it would be an easy thing to add, but beyond this PR. In that case I would wait until the LabelEncoder is implemented in Machine (or work on it in another pr)

[geektoni]

Ah yes, it isn't implemented, even though it would be an easy thing to add, but beyond this PR. In that case I would wait until the LabelEncoder is implemented in Machine (or work on it in another pr)

I see. @Hephaestus12 could you please move the new data files and the glm meta example on another PR and remove them from here? Then, could you also change this PR from ”draft” to the normal mode?

We can then have a final look and if tests pass we merge.

[geektoni]

This is not needed anymore, right?

[Hephaestus12]

I'll have to make another function public but yes, we can remove the friend class.

[geektoni]

Looks good to me. MacOS failures seem unrelated to this patch. Once the rest is green (and the above comment is resolved) we can merge.

[gf712]

is there a reason why the features are added to the state here?

[Hephaestus12]

No particular reason, I did it this way as it was done like this in LinearMachine:

shogun/src/shogun/machine/LinearMachine.cpp

Lines 70 to 76 in ec557d8

	if (data)
	{
	if (!data->has_property(FP_DOT))
	error("Specified features are not of type CDotFeatures");
	set_features(std::static_pointer_cast<DotFeatures>(data));
	}

[Hephaestus12]

Should I remove it?

[gf712]

I am not sure, you'll have to see if this has side effects that you rely on. @LiuYuHui do you know if this is required? Because you had to refactor this code recently in the feature branch

[gf712]

this should be fixed at some point. I guess when we have the LabelEncoder in Machine, otherwise you are performing a potentially expensive operation in each iteration

[LiuYuHui]

it seems like that RegressionLabels is what LabelEncoder lacks, currently, LabelEncoder only support MulticlassLabels and BinaryLabels.

[gf712]

True, but it should be possible to add it?

[geektoni]

If you want to remove code, just remove it. Don’t just comment it out. Please don’t commit this kind of changes.

[Hephaestus12]

Comment removed

[gf712]

lgtm, can merge it now and do minor fixes related to the machine refactor later on

[Hephaestus12]

Can we merge this?

[gf712]

Merging this so that @Hephaestus12 can use it to continue working on the influenza app. Any fixes, like cleaning up the distributions and optimiser, can be done in the future. Thank you!

[Hephaestus12]

Yaay 💯