ossia / libossia

A modern C++, cross-environment distributed object model for creative coding and interaction scoring
https://ossia.io
GNU Lesser General Public License v3.0
207 stars 34 forks source link

pushing nan to a float parameter results in invalid json #803

Open x37v opened 2 years ago

x37v commented 2 years ago

{"FULL_PATH":"/float","TYPE":"f","VALUE":NaN,"ACCESS":3,"CLIPMODE":"none"}

rapidjson seems to be okay with this but other json parsers don't accept NaN without quotes as valid json

TEST_CASE("test_json_nan", "test_json_nan")
{
  using namespace std::literals;
  generic_device serv{"foo"};
  TestDeviceRef dev{serv};

  dev.float_addr->push_value(std::numeric_limits<float>::quiet_NaN());
  auto json = oscquery::json_writer{}.query_namespace(dev.float_addr->get_node());
  std::cout << json.GetString() << std::endl;
}
jcelerier commented 2 years ago

Hmw so while trying a fix to the JSON library we're using, rapidjson, to put those in quotes (https://github.com/jcelerier/rapidjson/tree/nan_inf_as_strings - for the OSCQuery needs it should work), I end up with some problem when trying to implement parsing: how is parseValue("[ \"NaN\" ]"); supposed to be treated ?

With the current implementation in the lib it would parse as a string which makes sense.. imagine someone uses NaN as a username, you don't want to start treating this as a double when parsing I assume.

But on the other hand being able to do a correct round-trip between FP representation and JSON is fairly important I believe?

jcelerier commented 2 years ago

Maybe we should just filter nans on the ossia side, what do you think ? I wonder if replacing them by zero would be a big correctness issue...

x37v commented 2 years ago

Maybe we should just filter nans on the ossia side, what do you think ? I wonder if replacing them by zero would be a big correctness issue...

I'm not sure.. maybe instead of 0 it would make sense to replace them with the lower or upper end of a range if that is defined, 0 otherwise?

jcelerier commented 2 years ago

ah yes, that makes sense:

  1. if there is a "default value" set on the parameter, use that
  2. use the range min
  3. use 0

I'm wondering where is the best place to put this: I think that as far as possible we should filter for NaNs on entry of ossia APIs to make sure that our data model is always safe. But we also have to filter on output to be safe as for instance, someone could set a range of min=0 max=0 by mistake, which would likely cause a NaN if there are some unchecked divisions by (max - min) in the code ; I remember spending a lot of time making sure these cases are always checked but FP math is fairly corner-case-y and not sure we got them all...