openmc-dev / plotter

Native plotting GUI for model design and verification
MIT License
43 stars 17 forks source link

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

Closed ecasglez closed 8 months ago

ecasglez commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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

ecasglez commented 9 months ago

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 commented 9 months ago

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

paulromano commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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:

ecasglez commented 9 months ago

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 commented 9 months ago

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 commented 8 months ago

Ok. I found it!

It is related to this issue in the pugixml parser: https://github.com/zeux/pugixml/issues/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 commented 8 months ago

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