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

Failing to read real numbers in the xml files in certain regional configurations #124

Closed
ecasglez opened this issue Oct 3, 2023 · 12 comments · Fixed by openmc-dev/openmc#2723

Comments

@ecasglez
Copy link

ecasglez commented Oct 3, 2023

My xml files contain real numbers written using "." as decimal separator, which is the standard when dealing with programming lenguages.

However, my QT installation is configured to use "," as decimal separator, which is the default when using Spanish regional configuration. When I run openmc-plotter I get this error:

 Reading materials XML file...
 ERROR: Need to specify a positive density on material 100.

But the density of my material 100 is positive: 0.9965 g/cm3. Since the plotter is trying to read that number using Spanish locales, it stops at the "." and reads only the 0, showing that error.

In order to make it run, I have to execute openmc-plotter like this:

LC_NUMERIC="en_US"; export LC_NUMERIC;  openmc-plotter

This way the numeric format is forced to be the en_US one, that is, using "." as decimal separator.

I tested it on Kubuntu 22.04. The xml files were created in the same machine using Spyder and Openmc, and they were written with "." automatically.

This only affects openmc-plotter, running openmc with those xml files works well without any change.

Openmc-plotter should be forced to always read files using en_US configuration.

@pshriwise
Copy link
Collaborator

Hi @ecasglez,

Thanks for reporting this! It's not something we've run into yet. I'm trying to figure out how tied to QT this problem is vs. it being something we can configure in OpenMC's Python API. Does the following work for you in the same environment?

$ python -c "import openmc; openmc.Materials.from_xml()"

@pshriwise
Copy link
Collaborator

Actually, scratch that. The plotter no longer reads XML files using the Python interface. It reads them using the openmc.lib module, the Python interface to the compiled library.

I'll take a look at how to set the locale for the application via QT.

@pshriwise
Copy link
Collaborator

Can you try this branch of the plotter out to see if it fixes this problem, @ecasglez?

@ecasglez
Copy link
Author

ecasglez commented Oct 3, 2023

I think it is still happening. I am not sure if I missed something. You can reproduce it with the test input of this repository:

  1. Go to folder plotter/tests/setup_test.
  2. Modify materials.xml of one material from 4.5 g/cc to a value lower than 1, like 0.5 g/cc
  3. Run the plotter like this:
LC_NUMERIC="es_ES.UTF-8"; export LC_NUMERIC;  openmc-plotter

I will try your changes again later.

@pshriwise
Copy link
Collaborator

Hrmm that's not causing an error for me.. I'll try to reproduce this another way maybe.

@paulromano
Copy link
Contributor

This should be fixed by openmc-dev/openmc#2574. @ecasglez if you're able to try out the develop branch of OpenMC, that should solve your problem.

@ecasglez
Copy link
Author

ecasglez commented Oct 4, 2023

It is still happening. When I run openmc-plotter it is using the right develop openmc:

                 | The OpenMC Monte Carlo Code
       Copyright | 2011-2023 MIT, UChicago Argonne LLC, and contributors
         License | https://docs.openmc.org/en/latest/license.html
         Version | 0.13.4-dev
        Git SHA1 | 61c43527e6a9f7af6d309d050e7dcf906a7013a2
       Date/Time | 2023-10-04 18:19:23
  OpenMP Threads | 2

 Reading settings XML file...
 Reading cross sections XML file...
 Reading materials XML file...
 ERROR: Need to specify a positive density on material 100.

However, if I just run openmc it runs well.

I will have a deeper loop on Friday.

@pshriwise
Copy link
Collaborator

Thanks for trying this again @ecasglez. I was thinking yesterday that it's strange for it to work with the executable but not the plotter as both OpenMC initializations should be relying on the same compiled library. A couple of questions:

  • How are you installing the OpenMC Python API?
  • Did you reinstall the Python API after updating to the develop branch and recompiling OpenMC?

@ecasglez
Copy link
Author

ecasglez commented Oct 4, 2023

First, I removed the folder of my previous openmc installation which was /opt/openmc_0.13 and I also typed:

pip3 uninstall openmc

Then I compiled from sources the develop branch following the instructions here but swiching to the develop branch before compilation typing:

git checkout develop

It is actually getting the develop branch since it is writing Version | 0.13.4-dev when running both in standalone and through the plotter.

I think QT is somehow forcing the decimal numbers to follow my regional configuration. When running openmc standalone I think no QT libraries are loaded, so there is no problem when reading the numbers.

@pshriwise
Copy link
Collaborator

pshriwise commented Oct 4, 2023

Thanks for specifying! That all sounds good to me. Always good to double-check though.

A sound theory about the QT libraries. Not something we've encountered before. I'll try to dig into it further soon. When you tried the experimental plotter branch linked above, you reinstalled the plotter in a similar manner?

@ecasglez
Copy link
Author

ecasglez commented Oct 6, 2023

Ok. I found it!

It is related to this issue in the pugixml parser: zeux/pugixml#469.

I am not sure if they are going to fix this in the upstring xml parser. In the mean time, the following patch for OpenMC solves it. I've tried it in the develop branch of OpenMC.

diff --git a/src/initialize.cpp b/src/initialize.cpp
index d667f4b03..a8990d7b5 100644
--- a/src/initialize.cpp
+++ b/src/initialize.cpp
@@ -43,6 +43,14 @@ int openmc_init(int argc, char* argv[], const void* intracomm)
 {
   using namespace openmc;
 
+  // This fixes parsing the xml files when running openmc-plotter in regions
+  // where the decimal separator is , insted of .
+  // This is an issue of the upstream xml parser pugixml.
+  // See https://github.com/zeux/pugixml/issues/469
+  if (std::setlocale(LC_ALL, "C") == NULL) {
+    fatal_error("Cannot set local to C.");
+  }
+
 #ifdef OPENMC_MPI
   // Check if intracomm was passed
   MPI_Comm comm;

@pshriwise
Copy link
Collaborator

Nice detective work and I'm glad it's working for you. Would you be up for submitting a PR for this?

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