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

Numbers in scientific notation are not tolerated #319

Closed
roncapat opened this issue Jun 7, 2022 · 8 comments
Closed

Numbers in scientific notation are not tolerated #319

roncapat opened this issue Jun 7, 2022 · 8 comments
Labels
question Further information is requested

Comments

@roncapat
Copy link

roncapat commented Jun 7, 2022

def check_sequence_type_is_allowed(sequence):
# Check if the items of the sequence aren't dissimilar.
# Also, check if the item type is one of bool, int, float, str.
if not sequence: # Don't allow empty sequences
return False
subtype = type(sequence[0])
if subtype not in (bool, int, float, str):
return False
for item in sequence:
if not isinstance(item, subtype):
return False
return True

This code rejects numeric lists containing numbers in scientific notation, like:
[1.0, 1e-9]

@clalancette
Copy link
Contributor

Are you sure about that?

$ python3 -c 'print(type(1e-9))'
<class 'float'>

Also, if I take that function, put it in a file, and then call it, like the following:

def check_sequence_type_is_allowed(sequence):
    # Check if the items of the sequence aren't dissimilar.
    # Also, check if the item type is one of bool, int, float, str.
    if not sequence:  # Don't allow empty sequences
        return False
    subtype = type(sequence[0])
    if subtype not in (bool, int, float, str):
        return False
    for item in sequence:
        if not isinstance(item, subtype):
            return False
    return True

print('Empty:', check_sequence_type_is_allowed([]))
print('Float:', check_sequence_type_is_allowed([1.0]))
print('Scientific:', check_sequence_type_is_allowed([1.0, 1e-9]))

I get:

Empty: False
Float: True
Scientific: True

What's the larger problem that you are having?

@clalancette clalancette added the question Further information is requested label Jun 7, 2022
@roncapat
Copy link
Author

roncapat commented Jun 7, 2022

The problem is in the ROS components parameter load chain. I think (but now I will give it a try) that scientific numbers are parsed from yaml config files as strings and not floats.

I encountered an error wile switching robot_localization package from being a node to being a component. Small covariances in config file (1e-9) started to throw the exception from evaluate_parameters.py

@roncapat
Copy link
Author

roncapat commented Jun 7, 2022

I printed yaml_evaluated_value variable prior to calling check_sequence_type_is_allowed() and got:

[1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, '1e-9', 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, '1e-9', 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, '1e-9', 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, '1e-9', 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, '1e-9']

see that 1e-9 is parsed as string in yaml?

The question that arises is: is this an invalid notation in yaml files?
Or are scientific notation values accepted but caution is needed in this package to correctly interpret them?

Note: launching robot_localization as upstream (so Node, not Component) does not trigger this behaviour.

@roncapat
Copy link
Author

roncapat commented Jun 7, 2022

I may have spotted the issue... the problem is with how I loaded the parameters from file to fill the dictionary to pass to ComposableNode. yaml.safe_load from yaml python package interprets the scientific numbers as strings, while the internal ROS2 yaml parsing logic handles them correctly.

I close, sorry for the noise.
I may add further comments if I get any other useful information for anyone having my same problem in future here.

@roncapat roncapat closed this as completed Jun 7, 2022
@clalancette
Copy link
Contributor

Ah yeah, I see. In short, this seems to be a bug in PyYAML: yaml/pyyaml#173 . Indeed, if I run the following:

python3 -c "import yaml ; print(yaml.safe_load('[1.0e-9, 1e-9]'))"

I get:

[1e-09, '1e-9']

@roncapat
Copy link
Author

roncapat commented Jun 7, 2022

Thanks for spotting the issue in the library! I am trying to figure it out (I work with C++ in ROS2 so I need a bit of time to figure out how to efficiently browse rclpy code) whether ROS2 rclpy Node implementation uses pyyaml or another implementation to read parameters from files.
Do you know more details about it?

@clalancette
Copy link
Contributor

uses pyyaml or another implementation to read parameters from files.

It uses PyYAML:

@roncapat
Copy link
Author

roncapat commented Jun 7, 2022

I digged in rclpy and if you pass a filepath from launch file, rclpy seems to use the rcl implementation (so bypassing PyYAML).

A simple string (filepath) brings to this switch:

# Value is a singular type, so nothing to evaluate

so it is passed "as it is" to the Node.

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

No branches or pull requests

2 participants