ros2cli icon indicating copy to clipboard operation
ros2cli copied to clipboard

'ros2 param load' fails when double parameter in parameter file is in scientific notation

Open bijoua29 opened this issue 2 years ago • 10 comments

Bug report

Required Info:

  • Operating System:
    • Ubuntu 22.04
  • Installation type:
    • binaries
  • Version or commit hash:
    • latest rolling release
  • DDS implementation:
    • N/A
  • Client library (if applicable):
    • ros2cli

Steps to reproduce issue

  1. Run an application that loads parameters from a parameter file, that includes a parameter specified using scientfic notation 5e-06, using
ros2 run parameter_test parameter_test --ros-args --params-file parameter_test/config/params.yaml
  1. Load parameters again from the same parameter file using
ros2 param load param_test parameter_test/config/params.yaml

Expected behavior

Parameters are loaded properly in step 1 Parameters are loaded properly in step 2

Actual behavior

Parameters are loaded properly in step 1 However, in step 2, it results in following error:

Set parameter test_double failed: Wrong parameter type, parameter {test_double} is of type {double}, setting it to {string} is not allowed.

Additional information

Attached is package that was used for test parameter_test.zip

bijoua29 avatar May 26 '23 15:05 bijoua29

I believe that root cause is python yaml does not support scientific notation.

https://github.com/ros2/rclpy/blob/20125a4de2d955948db9ff58810efd03cb5603f3/rclpy/rclpy/parameter.py#L194

fixed with https://github.com/yaml/pyyaml/issues/173

fujitatomoya avatar Jun 07 '23 06:06 fujitatomoya

@fujitatomoya I don't believe this to be the issue. I have tried 5.0e-06 and get the same error message.

bijoua29 avatar Jun 07 '23 14:06 bijoua29

I don't believe this to be the issue. I have tried 5.0e-06 and get the same error message.

what do you mean? i think this is because of https://github.com/yaml/pyyaml/issues/173, and could be addressed by https://github.com/yaml/pyyaml/pull/174/files. are you saying that this https://github.com/ros2/ros2cli/issues/834 has another issues?

fujitatomoya avatar Jun 07 '23 17:06 fujitatomoya

Maybe I'm missing something. My understanding was that pyyaml#174 fixes the issue where scientific notation without a dot in the mantissa is not resolved as a float but as a string. What I am saying is that in my repro steps above, I changed the parameter to have a dot in the mantissa and it still resolves it as a string. I'm not sure where the issue is, with respect to #834, but it exists regardless of whether there is a dot or not in the mantissa.

bijoua29 avatar Jun 07 '23 17:06 bijoua29

@bijoua29 thanks for the explanation.

fujitatomoya avatar Jun 07 '23 17:06 fujitatomoya

I'm also not able to reproduce this with standard ROS 2 tooling.

That is to say, I took your code and removed the references to generate_parameter_library, and just used a regular declare_parameter. You can see exactly what I did in https://github.com/clalancette/parameter_test .

With that setup, your tests above work just fine for me, in Rolling, Iron, and Humble. Can you check to confirm that the same thing works for you?

clalancette avatar Jun 15 '23 19:06 clalancette

@clalancette I ran your modified package with standard ROS 2 tooling i.e. without generate_parameter_library, and it still fails for me on the latest Rolling.

That is, the following works fine:

ros2 run parameter_test parameter_test --ros-args --params-file parameter_test/config/params.yaml

but the following fails after the node is launched:

ros2 param load param_test parameter_test/config/params.yaml

with the error:

Set parameter test_double failed: Wrong parameter type, parameter {test_double} is of type {double}, setting it to {string} is not allowed.

Interestingly, if I have 0.00006 (no scientific notation) in the parameter file, it succeeds when launching but still fails on the subsequent param load.

I am now curious what is different about your setup from mine.

bijoua29 avatar Jun 18 '23 16:06 bijoua29

but the following fails after the node is launched:

ros2 param load param_test parameter_test/config/params.yaml

You are correct, that is failing. Thanks for pointing it out.

That said, this does look to be a consequence of https://github.com/yaml/pyyaml/pull/174 not being in yet. I locally modified my PyYAML sources to have that PR in place, and with that there, the ros2 param load works properly. So I think we are more-or-less waiting on that one to be merged and released.

clalancette avatar Jun 21 '23 17:06 clalancette

@clalancette Thanks for confirming my findings.

I am not hopeful that yaml/pyyaml#174 is going to be merged anytime soon since it has been open for 5 years. So, did you install pyyaml locally using 'setup.py install'? Not sure how I could automate that since I am running in a docker container.

bijoua29 avatar Jun 22 '23 07:06 bijoua29

So, did you install pyyaml locally using 'setup.py install'?

No, I edited the system version of resolver.py locally to add in the code from that PR to test with.

clalancette avatar Jun 22 '23 12:06 clalancette