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

[🐛 bug report] XML parsing of float values seems to respect current locale #499

Open
jungbluthb opened this issue Sep 28, 2021 · 4 comments

Comments

@jungbluthb
Copy link

Summary

If I try to run the inverse rendering example from the documentation (https://mitsuba2.readthedocs.io/en/latest/src/inverse_rendering/advanced.html), I run into issues during scene loading. It seems like the parser for float values respects my current locale (de.DE) and therefore isn't able to parse the give floating point numbers which are using a point as decimal separator.

System configuration

  • Platform: openSUSE Leap 15.2
  • Compiler: g++-10 (SUSE Linux) 10.2.1 20200825 [revision c0746a1beb1ba073c7981eb09f55b3d993b32e5c]
  • Python version: Jupyter console 6.4.0, Python 3.9.7 | packaged by conda-forge | (default, Sep 23 2021, 07:28:37), IPython 7.27.0
  • Mitsuba 2 version: d4d20ea ("Fix doc image generation script", 2021-08-27)
  • Compiled variants:
    • scalar_rgb
    • scalar_spectral
    • packet_rgb
    • gpu_autodiff_rgb

Description

During testing I run into issues while loading scenes from xml-files provided in ressources. The float-values defined in the files use a dot as decimal separator. This leads to runtime errors during parsing:

RuntimeError: ​[xml.cpp:223] Error while loading "res/mitsuba2/test_mitsuba2_inverse_rendering_01.xml" (at line 37, col 4): could not parse floating point value "28.8415".

As far as I can see xml.cpp uses "stof" for conversion which seems to respect the current locale. In my opinion this should not happen, since the xml scene description has a fixed format which does not seem to depend on the current system locale. I can circumvent this issue by rewriting all floats to the expontial notation (e.g. 10e-1), but doing this for all floats is quit tedious and error prone. Maybe someone could fix this or point me into the right direction, if I am missing something on my side. Thank you in advance.

Steps to reproduce

scene = load_file('~/mitsuba2/resources/data/docs/scenes/shape_ply_bunny.xml')

@jungbluthb jungbluthb changed the title [🐛 bug report] XML parsing float values seems to respect current locale [🐛 bug report] XML parsing of float values seems to respect current locale Sep 28, 2021
@merlinND
Copy link
Member

Hi @jungbluthb,

I could reproduce the issue on my machine by adding the following at the start of main() in mitsuba.cpp:

    std::locale l1("de_DE.UTF-8");
    std::locale::global(l1);

And then running:

mitsuba ../resources/data/scenes/cbox/cbox.xml

Could you please check if adding the following fixes the issue?

    const std::locale &l1 = std::locale::classic();
    std::locale::global(l1);

(Note that this won't affect scenes loaded from Python, we'll need to find a better place to set the locale in all cases if we decide to adopt this fix).

@jungbluthb
Copy link
Author

Hello @merlinND,

thank you for your reply. Unfortunately it seems that the problem is caused by my development environment (conda venv with
Emacs, elpy ...). If I use ipython from the command-line, everything works flawless although the output of locale looks like this:

LANG=de_DE.UTF-8
LC_CTYPE=de_DE.UTF-8
LC_NUMERIC="de_DE.UTF-8"
LC_TIME="de_DE.UTF-8"
LC_COLLATE="de_DE.UTF-8"
LC_MONETARY="de_DE.UTF-8"
LC_MESSAGES="de_DE.UTF-8"
LC_PAPER="de_DE.UTF-8"
LC_NAME="de_DE.UTF-8"
LC_ADDRESS="de_DE.UTF-8"
LC_TELEPHONE="de_DE.UTF-8"
LC_MEASUREMENT="de_DE.UTF-8"
LC_IDENTIFICATION="de_DE.UTF-8"
LC_ALL=

I can only reproduce using your hint from above. I think I have to check my environment or stick to python dictionaries for scene creation. Sorry for the confusion.

@merlinND
Copy link
Member

merlinND commented Oct 5, 2021

Interesting!
Feel free to reopen if you find out a fix is needed on the Mitsuba side.

@merlinND merlinND closed this as completed Oct 5, 2021
@wjakob
Copy link
Member

wjakob commented Oct 5, 2021

Let's leave this one open. It would be good to replace the parsing routines with something guaranteed to be locale-independent.

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

No branches or pull requests

3 participants