Extend the meta-language to support keyword arguments

[sorig]

Allows keyword arguments in class constructors and factory functions (when the factory function is called to set the value of a newly initialised variable).

The update in meta-language allows translation of statements like these (meta-language):

Kernel k = kernel_machine("SomeKernel", some_parameter=2)

Kernel k2(parameter=500)

A) To something like this (python)

k = kernel_machine("SomeKernel")
k.put("some_parameter", 2)

k2 = Kernel()
k2.put("parameter", 500)

B) Or something like this (python):

k = kernel_machine("SomeKernel", some_parameter=2)

k2 = Kernel(parameter=500)

Changing from translation type A) to B)

To get A), the targets/python.json file should contain:

{
    ...
    "Init": {
        "Construct": "$name = $typeName($arguments)\n$kwargs",
        "Copy": "$name = $expr\n$kwargs",
        "KeywordArguments": {
            "List": "$elements",
            "Element": "$name.put(\"$keyword\",$expr)",
            "Separator": "\n",
            "InitialSeperatorWhenArgs>0": false
        },
        ...
    },
    ...
    "Expr": {
        ...
        "GlobalCall": "$method($arguments)",
        ...
    },
    ...
}

And to get B), the targets/python.json should contain:

{
    ...
    "Init": {
        "Construct": "$name = $typeName($arguments$kwargs)",
        "Copy": "$name = $expr",
        "KeywordArguments": {
            "List": "$elements",
            "Element": "$keyword=$expr",
            "Separator": ", ",
            "InitialSeperatorWhenArgs>0": true
        },
        ...
    },
    ...
    "Expr": {
        ...
        "GlobalCall": "$method($arguments$kwargs)",
        ...
    },
    ...
}

Where do keyword arguments not work?

Keywords arguments cannot be used anywhere else than in class constructors to initiliase variables and global functions to initialise variables. See the following examples:

# Keyword arguments are supported when constructing objects

# Either when using factory functions:
Kernel k = kernel_machine(a=2)
Kernel k2 = kernel_machine("GaussianKernel", c=1.0)

# Or when constructing objects from class constructors
GaussianKernel k3(1, a=2, b=3)

# This fails (keyword arguments appear before normal arguments)
#Kernel k4(asd=123,"SomeParam",kw=val)

# This fails (keywords not allowed outside initialisations of variables. This is a re-assignment of a previously initialised variable)
#k4 = kernel_machine("GaussianKernel", b=2)

# This fails (keywords not allowed outside initialisation of variables. This is a nested kwarg, but it doesn't appear in the call that initialises `k6` directlry)
#Kernel k6 = kernel_machine("GaussianKernel", a=glob_fun(kw=123))

# This is fine:
Kernel k6 = kernel_machine("GaussianKernel", a=glob_fun(ordinary_argument))

# This also fails (both kwargs appear outside of variable initialisation)
#kernel_machine("GaussianKernel", a=glob_fun(kw=123))

[sorig]

@karlnapf Can we get a real example of this instead of my examples/meta/src/meta_api/kwargs.sg file? That would make it easier to change all the other target .json files (and see what edge cases I've missed)

[karlnapf]

Ah this is great, thanks for that!

I will review a bit in my phone as I am travelling

[karlnapf]

As for real examples, keep in mind that the kwargs.sg file is translated and executed.
I think it should fail as you’re setting parameters that don’t exist....

I think creating a few objects, mimicking their constructors, in your file should re real enough?

[karlnapf]

Can’t see the travis logs but all jobs failed which is due to the putting Parameter c of a Gaussian kernel

[karlnapf]

You can both cover the current constructors and a factory method

[sorig]

Yeah I know, but they would fail anyway since I've only added Python support for now.

Will update with a real example and see how it goes

[karlnapf]

Sweet! I would start with making an existing ctor work and then do the factory method next

[karlnapf]

Not sure about this capitalisation underscore mix

[sorig]

haha yeah you're right. Will make this consistent

[sorig]

Initial underscores t_ and p_ are enforced by the parsing library, that's why there is some confusion in this file

[sorig]

This piece of python code

k2 = GaussianKernel()
k2.put("lhs_equals_rhs", True)

Throws the following error

386/392 Test #364: generated_python-meta_api-kwargs ........................................................***Failed    0.20 sec
Traceback (most recent call last):
  File "/opt/shogun/build/examples/meta/python/meta_api/kwargs.py", line 44, in <module>
    k2.put("lhs_equals_rhs", True)
RuntimeError: [ERROR] In file /opt/shogun/src/shogun/base/SGObject.h line 349: Type for parameter with name "lhs_equals_rhs" is not correct.

while this piece of C++ code is fine

auto k2 = some<CGaussianKernel>();
k2->put("lhs_equals_rhs", true);

lhs_equals_rhs is declared as a C++ bool in CKernel. Is this a problem with a type map somewhere?

---

UPDATE: I changed the example to use integers instead of booleans. It looks like there is an underlying problem with booleans and some of the interfaces. This may need to be investigated. I removed the boolean parameter from the example as changing to integer literals made the C++ version fail

[karlnapf]

@lisitsyn here is someone using tags, so maybe you can help?
I’ll look back into from friday

[sorig]

Similarly, as the only language octave is complaining about:

k = GaussianKernel();
k.put("log_width", 2.0);

throwing this error:

[ERROR] In file /opt/shogun/src/shogun/base/SGObject.h line 349: Type for parameter with name "log_width" is not correct.
error: C++ side threw an exception of type shogun::ShogunException
error: called from
    /opt/shogun/build/examples/meta/octave/meta_api/kwargs.m at line 37 column 1

while the equivalent code in all other languages works fine

[sorig]

@lisitsyn Here's an issue related to the SWIG interfaces and the parameter framework. In python, I can't see what the parameters of an object is:

In [1]: from shogun import GaussianKernel

In [2]: k = GaussianKernel()

In [3]: k.parameter_names()
Out[3]: <Swig Object of type 'std::set< std::string > *' at 0x11408e458>

Which means I have to dig around in C++ code to find the parameter names. Can this be fixed?

[vigsterkr]

I reckon a .i include missing in our swig descriptors for typemapping std::set

…

On 29 Jan 2018, at 19:17, Esben Sørig ***@***.***> wrote: @lisitsyn Here's an issue related to the SWIG interfaces and the parameter framework. In python, I can't see what the parameters of an object is: In [1]: from shogun import GaussianKernel In [2]: k = GaussianKernel() In [3]: k.parameter_names() Out[3]: <Swig Object of type 'std::set< std::string > *' at 0x11408e458> Which means I have to dig around in C++ code to find the parameter names. Can this be fixed? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[vigsterkr]

@sorig i've just tried to do that what i said above.... it's not working... i'll let u know if i have found a solution

[vigsterkr]

@sorig ok i've managed to do this:

>>> import shogun
>>> k = shogun.GaussianKernel()
>>> k.parameter_names()
('cache_size', 'combined_kernel_weight', 'lhs', 'lhs_equals_rhs', 'log_width', 'm_distance', 'm_precomputed_distance', 'normalizer', 'num_lhs', 'num_rhs', 'opt_type', 'optimization_initialized', 'properties', 'rhs')

i guess this is what you would like to have right? :)

[sorig]

@vigsterkr Amazing, thanks! Crucial functionality for people working from the swig interfaces

[vigsterkr]

@sorig lemme push it to develop

[vigsterkr]

3cc0e37 is the patch that gets u the above mentioned functionality

[vigsterkr]

we have actually a problem: std::set isn't supported in the following interfaces: lua, java, set, r, octave, c#.
in other words wrapper for std::set is only available in case of python, ruby, in the rest of the case the api will not be available... which is far from optimal so we should actually return our own CSet :D yaaay for custom things :(

[sorig]

Yeah, it looks like those interfaces just broke with the change. Is it really necessary to have std::set exposed on the interfaces? It seems to me that parameter_names would be absolutely fine as a std::list std::vector. Are there examples in the code base where std::set is absolutely necessary?

[vigsterkr]

@sorig if we wanna switch we should then switch the interface to use std::vector<std::string> as that's the only typemap that is available in all of the languages that we support (std::list is not available in c# and lua)

[vigsterkr]

@lisitsyn what do you think? custom CSet<std::string> or std::vector<std::string> as return value for parameter_names()?

[lisitsyn]

@vigsterkr std::vector for sure!

[vigsterkr]

@lisitsyn do you have any clue why

%include "std_vector.i"

is surrounded by #if !defined(SWIGJAVA) ?
i.e. why no java typemaps for

    %template(IntStdVector) vector<int32_t>;
    %template(DoubleStdVector) vector<float64_t>;

?

[vigsterkr]

commit 5423ead
Author: Christian Widmer cwidmer@tuebingen.mpg.de
Date: Fri Jun 15 17:29:53 2012 +0200

included std_vector typemap while avoiding java compile error

one would hope that there's no more compiler error, or? :D

[lisitsyn]

Who knows.. :)

[vigsterkr]

@sorig 532a8d4 should be the thing for you :)

[sorig]

Great, thanks a lot!

[sorig]

All green except for Octave (see error description above) which seems to be a bug with the octave interface and float types. I'm not currently able to install octave on my macine to figure out what the issue is. Is there a server somewhere where I can test all the interfaces?

It looks like there are issues with basic types in several of the interface languages. Would be good to write a thorough test case once this PR is merged.

[karlnapf]

why two kernels?

[sorig]

One of them had a boolean keyword argument earlier, but that fails in python. I’ll revert it and comment it out

[karlnapf]

Real example

[karlnapf]

Cool!

I suggest you comment out everything in the API example that currently fails, and then we merge this.
Then I can take over and fix the interface issues ...

[sorig]

OK, included last review changes. This is ready to be merged

[karlnapf]

Excellent, thanks!