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

Required and Optional parameters in initialiser #561

Open
christian-rauch opened this issue May 10, 2019 · 6 comments
Open

Required and Optional parameters in initialiser #561

christian-rauch opened this issue May 10, 2019 · 6 comments
Assignees
Labels

Comments

@christian-rauch
Copy link
Collaborator

The initialiser definition supports Required and Optional parameters.

What is the intended behaviour of those?
The Optional can be used with and without default argument (set via =). Required on the other hand does not support default arguments but the Initializer can be constructed without providing an argument value, in which case it is using random memory.

I think the Required and Optional properties should be implicitly defined by a default argument:

  • if a default argument is set via =, the parameter is Optional
  • if no default argument is given, the parameter is Required and the initialiser should throw an error if no argument is provided.
@VladimirIvan
Copy link
Collaborator

Required means that the user has to provide a value (no default value is defined by design). The initialiser will throw if a required parameter is not set.
Optional parameter will always have a value defined, hence no reason to throw any errors since default value can be used.

Use optional parameters for default thresholds, convergence rates, ...
Use required parameters for task/problem specific values, e.g.: goals, link names, file names, ...
This is a mechanism to prevent initialisation failure due to human error. specifying a default file name would make it easy to accidentally load the default file when the parameter is not specified. The required flag will force the user to specify it if it is required.

@christian-rauch
Copy link
Collaborator Author

You can compile an Initialiser with no default argument for an Optional parameter. E.g. just define the Tolerance in IKsolver as Optional double Tolerance;. It will compile without issues. There is no meaning behind this. An Optional parameter must be required to have a default argument.

And further, if an Optional must have a default argument and a Required parameter must not have a default argument, then both are defined implicitly by providing a default argument or not.

I was also able to construct an Initialiser without providing a value for a Required parameter.

@VladimirIvan
Copy link
Collaborator

You can compile an Initialiser with no default argument for an Optional parameter.

If unspecified, default constructor is used. In case of double the value won't be set and you can indeed get garbage values. This is a bug (implemented here).

And further, if an Optional must have a default argument and a Required parameter must not have a default argument, then both are defined implicitly by providing a default argument or not.

True, but it is a lot easier to miss a required parameter if doesn't have an explicit keyword in front of it.

I was also able to construct an Initialiser without providing a value for a Required parameter.

Parameters are checked when the generic initialiser is casted to its specialised counterpart. This normally only takes place when you initialize a class with it. All initialisers in python are generic so they won't throw errors until you use them to initialise the class they are meant for.

@christian-rauch
Copy link
Collaborator Author

I have the feeling we should make the default argument explicit and not rely on a default constructor. And then all parameters without default argument are required to be set by a user.

Well, the Required argument is only checked when initialised via the XML loader or in C++. When using dicts in python, they are not checked. I had the issue that I forgot the set a value for a required parameter and ended up using random values.

@christian-rauch
Copy link
Collaborator Author

This is a minimal example example_ik_p2l.py.txt that defines a PointToLine task map:

    p2l_map = \
        ('exotica/PointToLine', {
            'Name': p2l_name,
            'EndEffector': [end_effector],
            # 'EndPoint': "0.5 0.5 0",  # this is a 'Required' parameter
        })

without the required EndPoint.
You can create the problem and provide it to the solver without any warnings about the undefined argument for parameter EndPoint.

@wxmerkt
Copy link
Collaborator

wxmerkt commented May 12, 2019

I think you uncovered a quite significant bug for when initialised from a Python tuple/dict or C++ map - but not from XML.

So the true question is: Why did IsSet() on the property in the value not thrown when ?

If we look in the generated point_to_line_initializer.h we see:

virtual void Check(const Initializer& other) const
    {
        if(!other.HasProperty("Name") || !other.properties_.at("Name").IsSet()) ThrowPretty("Initializer PointToLineInitializer requires property Name to be set!");
        if(!other.HasProperty("EndPoint") || !other.properties_.at("EndPoint").IsSet()) ThrowPretty("Initializer PointToLineInitializer requires property EndPoint to be set!");
    }

I think in this case the Property logic is broken when initialised from a Python or C++ dict but not from XML. Perhaps there is a difference in how they are created (the particular initialiser used rather than a generic or when they get passed/cast around?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants