Generic CSGObject methods

[gf712]

Now that all model parameters are stored in the same map and are aware of their function in the ML algorithms we can use this to have more generic getters that use AnyParameterProperties. In other words, can start replacing things like CSGObject::get_modelsel_names with a generic CSGObject::get_names(ParameterProperties pprop), which can be used for hyperparameters, gradients, and so on.
I think the best is to filter the ParametersMap with a filter method inside self and then use this inside each getter.

[karlnapf]

I think the mentioned method can be dropped.

As for the filter, I would just use the existing parameter_names method (or whatever the name is), and give it a filter parameter which has a default value.

Then we can have special API sugar for things like hyper-parameters, things that might be needed frequently by users

Since we are changing APIs all the time anyways atm (need to do a new release soon), we can definitely try to make this as clean as possible via dropping old things and chose appropriate names and patterns

[karlnapf]

this just as an internal filter method?
Sounds like a good idea to me!
Merge?

[karlnapf]

stuff like this should though be hidden from outside (as done here)

[gf712]

Yup, should work fine, just need to write a test for it!

[gf712]

Ah ok! So drop the get_modelsel_names and have a filter-able parameter_names? What about the print_*, that is still useful right and should be refactored no?

[karlnapf]

I would drop print, see other thread.
Yeah just parameter_names ... and maybe hyperparameter_names?

[karlnapf]

Actually I would name them

• parameters(bitmask filter with default returning everything)
• model_parameters()
• hyper_parameters()

[karlnapf]

"names" is redundant when we return a string list

[karlnapf]

Drop :)

On Thu, 13 Dec 2018 at 13:25, Gil ***@***.***> wrote: Ah ok! So drop the get_modelsel_names and have a filter-able parameter_names? What about the print_*, that is still useful right and should be refactored no? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#4432 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAqqv0aQIdcSuFJ6yO4EyylwdZEbswbSks5u4kdBgaJpZM4ZRc19> .

-- Sent from my phone

[gf712]

OK! That makes sense!

[gf712]

I am thinking if it makes sense to change ParameterProperties::NONE to be non zero, so that it is possible to filter parameters that are ParameterProperties::NONE or filter parameters that are none and gradient (ParameterProperties::NONE | ParameterProperties::GRADIENT), or any other combination with none. Right now this can't be done because 0 is in the same bit position as 1, basically meaning that ParameterProperties::NONE is the opposite of ParameterProperties::GRADIENT, whereas I think ParameterProperties::NONE should be a property.

[karlnapf]

I think there might be a problem as properties then might contradict, i.e. NONE and something else is set.
Those summarising property states are better computed on the fly imo. We could maybe offer a method for this?

[gf712]

I am not sure I understand what you mean with "Those summarising property states are better computed on the fly imo"

[karlnapf]

Sorry :)
What I meant was: NONE is a summarising property which can be inferred from knowing all the others. And I think this is why we shouldn’t encode this in a bit but rather compute it on the fly

[gf712]

OK! I am just thinking how a user can find several masks at once, including NONE. If NONE is 0, then a user would need to find NONE specifically and then find any other flag separately. Maybe a bit of code explains this better than I can.
What I would expect in a API to work:

// user wants all the names of the parameters that are NONE or GRADIENT
auto none_or_grad_params = obj.parameter_names(ParameterProperties::NONE | ParameterProperties::GRADIENT);

but that isn't possible with the current design. Right now we would need this:

// user wants all the names of the parameters that are NONE or GRADIENT
auto none_params = obj.parameter_names(ParameterProperties::NONE);
auto grad_params = obj.parameter_names(ParameterProperties::GRADIENT);

On the other hand this is possible:

// user wants all the names of the parameters that are GRADIENT or HYPER
auto hyper_or_grad_params = obj.parameter_names(ParameterProperties::HYPER | ParameterProperties::GRADIENT);

Maybe I am thinking about the API design differently to as what it is intended to be?

[karlnapf]

I realise now that indeed this does not make any sense. But making NONE nonzero also doesnt make sense (due to semantical conflicts when NONE and other flags are set).
I think the use case where a user wants parameters with no properties set OR some others set is not very realistic. Why not just drop the NONE from the API?

[gf712]

Closing this in favour of feature/correct-initials branch