Assertion failed (bb.isValid()) when opening eclipse case

[opm]

Assertion failed (bb.isValid()) when opening eclipse case

[JacobStoren]

I could not reproduce this by simply reading the provided file into ResInsight 0.9.24 (windows). Which Version of ResInsight are you using ?
The assert you have received indicates that for some reason an empty geometry has been created (and not rejected) and sent into the render pipeline. The file, however, does not look like it should be able to stress ResInsight in that respect.

To be continued I guess...

[stubbscroll]

I used the newest commit (version 0.9.20) which I built using the instructions in the README. I also tried a quite random selection of older commits (sorry, don't remember which ones) with the same result.

[JacobStoren]

I tried RH5 0.9.25 (which is basically bleeding edge version) and could not reproduce it either.
Is it only this particular model, or does all grids fail ?

[stubbscroll]

I have tried on another model (in addition to the one mentioned in the original post) with the same result. If you could kindly point me to other eclipse files I'd happy to test these.

[magnesj]

We have a few synthetic demonstration models available from the Debug menu. These models do not have anything with Eclipse interface reading. Try these and see if you can produce any geometry at all. Also try to uncheck "Use Shaders" fom the Edit->Preferences menu.

[stubbscroll]

I have now tested with all the debug models. The first three works fine; I'm able to view them and navigate around. The last one (input mock model) gave various results. The first few times I loaded this model, the program aborted with the following error:

ERROR: Program object identifier does not correspond to a shader program object.
-func: bool cvf::ShaderProgram::validateProgram(cvf::OpenGLContext*) const
-file: cvfShaderProgram.cpp(383)
Assertion failed: (validateProgram(oglContext)), file [home dir redacted]/STATOIL/code/ResInsight/VisualizationModules/LibRender/cvfShaderProgram.cpp, line 271
Aborted (core dumped)

I then proceeded to experiment with the preferences, especially "use shaders". From there on, this debug model loaded fine (although nothing is shown, unlike the other debug models). It might have been an OpenGL hiccup of some kind; either way I haven't managed to reproduce the above error.

Turning on/off "use shaders" and a couple of other options did not affect the original error. I get the same error message as in the first post for the provided model.

[stubbscroll]

So I finally took some time to investige this thorougly, and I managed to find the cause of the bug. According to the locale settings (Norwegian) on my computer, the decimal separator is comma (not period). This causes the input file to be parsed wrongly, as scanf expects a comma, but encounters a period. This in turn causes the model coordinates to be NaN (actually x and y are set to NaN, and z is set to some very small negative value which is displayed as -0), and the min/max values (x, y) of the bounding box are not updated, causing the bounding box to be illegal (min > max).

Somewhere in the program (probably in libqt) setlocale() is called with the locale settings from my computer. The sscanf function that fails is located in ResInsight/ThirdParty/Ert/devel/libecl/src/ecl_kw_grdecl.c in the function fscanf_alloc_grdecl_data().

I was able to load the dataset mentioned in the top post using the following dirty fix: Add setlocale(LC_ALL,"C") as the first statement of the function mentioned in the previous paragraph. In the same manner, my original bug can be reproduced by setting a locale with a "wrong" decimal separator: setlocale(LC_NUMERIC,"nb_NO.UTF-8") (remember to include locale.h).

It's possible that other file reading functions suffer from the same issue. I have only tested the import of *.grdecl files.

As for ways to fix it: Setting locale at the beginning of fscanf_alloc_grdecl_data() and restoring the old one before exiting looks like a rather dubious solution. I guess another solution is to write a locale-independent parser. I'll let the maintainers decide upon the course of action (@magnesj, @JacobStoren).

[joakim-hove]

As for ways to fix it: Setting locale at the beginning of
fscanf_alloc_grdecl_data() and restoring the old one before exiting looks
like a rather dubious solution. I guess another solution is to write a
locale-independent parser. I'll let the maintainers decide upon the course
of action (@magnesj https://github.com/magnesj, @JacobStorenhttps://github.com/JacobStoren
).

Ahh - ok; I guess I should be included among the guilty. I really don't
know what is the best solution here. Is it an option to force
setlocale(LC_ALL , "C") from top level?

Joakim

[stubbscroll]

I'm not sure if setlocale(LC_ALL , "C") at the top level will work, as I don't know where/when the locale is changed. I'm not familiar with lib-qt, but I suspect it is the culprit (from a quick glance at the call graph produced by valgrind), possibly when it's initialized. Also, setting the locale at the top level could possibly change the behaviour of the program if the locale settings are used, for instance for displaying dates and decimal numbers.

For me it feels somewhat inappropriate to change the parser in ecl_kw_grdecl.c, as it is part of a third-party library (libecl) which seemingly assumes that the behaviour of scanf is not changed by locale. Another option is to change the locale just before loading the input file (just before the external library is called), and revert to the old locale after returning.

[rolk]

Another option is to change the locale just before ... and revert to the old locale after

You can do that with:

const char* old_locale = strdup (setlocale (LC_NUMERIC, NULL));
setlocale (LC_NUMERIC, "C");
...
setlocale (LC_NUMERIC, old_locale);
free (old_locale);

@joakim-hove Note for opm-parser that you should imbue the stream that is reading from a deck; I don't think Eclipse ever handles localized numbers anyway.

[rolk]

You can do that with [setlocale]

Or, if you want to do it "by the book", you can (1) on Linux, define a scanf_l function that takes an additional locale_t argument, and wrap a regular scanf in a uselocale (setting the thread-local locale; that should have very little performance footprint) combo which sets and unsets the locale to the one specified, and (2) on Win32 and BSD use scanf_l/scanfl which already does this. Create the "C" locale with newlocale (POSIX/BSD) or _create_locale (Win32). It can probably be located (no pun intended) in its own compilation unit to keep the noise down.

[magnesj]

Nice digging, @stubbscroll !

We have experienced locale relate issues on the application level in ResInsight too. To be sure what locale the application is working in, we define the locale by using QLocale::setDefault(QLocale(QLocale::English, QLocale::UnitedStates)); in RiaMain.cpp.

This setting is obviously not propagated to ERT, as this is a Qt setting affecting only Qt functions. I would prefer the ERT locale issues to be handled internally in ERT, and then move this bug to ERT if nobody raises objections.

[magnesj]

We have done some more digging into this issue, we have concluded that the locale settings must be controlled by the application. Snippet from Qt doc:

Locale Settings

On Unix/Linux Qt is configured to use the system locale settings by default. This can cause a conflict when using POSIX functions, for instance, when converting between data types such as floats and strings, since the notation may differ between locales. To get around this problem, call the POSIX function setlocale(LC_NUMERIC,"C") right after initializing QApplication or QCoreApplication to reset the locale that is used for number formatting to "C"-locale.

Our fix by using QLocale is valid for all functions parsing text using Qt classes. In addition we will fix the issues for ERT in line with the workaround described in Qt doc.

[JacobStoren]

And a note to us all is that:
If Qt is used in an application together with ert on Linux, and this Application is run on a system with locale setting different enough from the standard C locale, ert functions will fail when reading ascii files.

[joakim-hove]

Our fix by using QLocale is valid for all functions parsing text using Qt
classes. In addition we will fix the issues for ERT in line with the
workaround described in Qt doc.

Do I interpret it correctly that the ERT code does not need immediate
updating for this issue?

[joakim-hove]

@joakim-hove https://github.com/joakim-hove Note for opm-parser that
you should imbue the stream that is reading from a deck; I don't think
Eclipse ever handles localized numbers anyway.

Thank you - I will look into "imbue(?)"; this is a topic I have not thought
of at all.

J