Cantera / cantera

Chemical kinetics, thermodynamics, and transport tool suite
https://cantera.org
Other
582 stars 342 forks source link

SolutionArray formatting tests fail with fmt 6.1.2 #1526

Closed speth closed 11 months ago

speth commented 1 year ago

Problem description

Using fmt version 6.1.2, a number of tests in the Python test suite that test the formatted output from SolutionArray report failures. This version of fmt is what is available on Ubuntu 20.04, so it would be nice to continue supporting it.

Steps to reproduce

  1. Compile Cantera using a system copy of fmt version 6.1.2
  2. Run scons test-python-composite

Behavior

The following tests fail:

============================================== short test summary info ===============================================
XFAIL test/python/test_composite.py::TestLegacyHDF::test_legacy_hdf_str_column - Unable to read fixed length strings from HDF
FAILED test/python/test_composite.py::TestSolutionArrayInfo::test_double_vector - AssertionError: assert 77 == 79
FAILED test/python/test_composite.py::TestSolutionArrayInfo::test_integer_vector - AssertionError: assert 79 == 81
FAILED test/python/test_composite.py::TestSolutionArrayInfo::test_long - AssertionError: assert 74 == 76
FAILED test/python/test_composite.py::TestSolutionArrayInfo::test_plus_minus_e - AssertionError: assert 79 == 81
FAILED test/python/test_composite.py::TestSolutionArrayInfo::test_plus_minus_f - AssertionError: assert 79 == 81
FAILED test/python/test_composite.py::TestSolutionArrayInfo::test_plus_minus_i - AssertionError: assert 73 == 77
FAILED test/python/test_composite.py::TestSolutionArrayInfo::test_scientific - AssertionError: assert 68 == 69
FAILED test/python/test_composite.py::TestSolutionArrayInfo::test_select_rows - AssertionError: assert 68 == 70
FAILED test/python/test_composite.py::TestSolutionArrayInfo::test_short - AssertionError: assert 68 == 70
FAILED test/python/test_composite.py::TestSolutionArrayInfo::test_string_vector - AssertionError: assert 77 == 79
FAILED test/python/test_composite.py::TestSolutionArrayInfo::test_strings - AssertionError: assert 76 == 78
FAILED test/python/test_composite.py::TestSolutionArrayInfo::test_water_extra - AssertionError: assert 22 == 24
FAILED test/python/test_composite.py::TestSolutionArrayInfo::test_water_simple - AssertionError: assert 16 == 18
====================================== 13 failed, 81 passed, 1 xfailed in 1.66s ======================================

A specific error that helps show that the output is indeed not as expected:

_______________________________________ TestSolutionArrayInfo.test_select_rows _______________________________________

self = <python.test_composite.TestSolutionArrayInfo testMethod=test_select_rows>

    def test_select_rows(self):
        arr = ct.SolutionArray(self.gas, 25)
        ix = [2, 5, 6, 9, 15, 3]
        arr2 = arr[ix]
>       self.check(arr2, arr2.info(width=self.width), 6)

test/python/test_composite.py:380: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <python.test_composite.TestSolutionArrayInfo testMethod=test_select_rows>
arr =       T          D   H2    H    O   O2   OH  H2O  HO2 H2O2   AR   N2
2.0 300.0  0.0818939  1.0  0.0  0.0  0.0  0.0  0.... 0.0  0.0
3.0 300.0  0.0818939  1.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0

[6 rows x 12 components; state='TDY']
repr = "      T          D   H2    H    O   O2   OH  H2O  HO2 H2O2   AR   N2\n2.0 300.0  0.0818939  1.0  0.0  0.0  0.0  0.0  ...  0.0\n3.0 300.0  0.0818939  1.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0\n\n[6 rows x 12 components; state='TDY']"
rows = 6

    def check(self, arr, repr, rows):
        count = 0
        width = None
        header = None
        for line in repr.split("\n"):
            if not len(line):
                break
            if width is None:
                width = len(line)
                assert width <= self.width
                header = line.split()
            else:
>               assert width == len(line)
E               AssertionError: assert 68 == 70
E                +  where 70 = len('2.0 300.0  0.0818939  1.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0')

test/python/test_composite.py:305: AssertionError

You can see that in the first data row, there is an ellipsis, not separated from adjacent entries, and also not present in either the header row or the following data row.

System information

Attachments

Complete log of running scons test-python-composite: test-python-composite-log.txt

ischoegl commented 12 months ago

There was a change of support libraries in {fmt} version 7.1.0. With the current version being 10.0 (and numerous existing workarounds in fmt.h for relatively dated versions), I am proposing to limit support to the most recent versions, while improving CI - see #1538. I found multiple bugs/regressions while addressing this issue report, which are mostly due to frequent changes of the {fmt} API.

ischoegl commented 12 months ago

Finally managed to get a configuration on macOS (Python 3.8 / fmt 6.1.2), by simply checking out the 6.1.2 hash from the submodule. It does, however, not compile with the following error right at the beginning:

For main:

g++ -o build/src/extensions/pythonShim.os -c -isystem /opt/homebrew/Caskroom/miniforge/base/envs/ctdev/lib/python3.8/site-packages/numpy/core/include -isystem /opt/homebrew/Caskroom/miniforge/base/envs/ctdev/include/python3.8 -isystem include/cantera/ext -isystem /opt/homebrew/Caskroom/miniforge/base/envs/ctdev/include -std=c++17 -Wno-deprecated-declarations -O3 -Wno-inline -g -Wall -fPIC -DNDEBUG -DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION -Iinclude -Iinclude -Ibuild/src src/extensions/pythonShim.cpp
scons: *** [build/ext/fmt/src/os.os] Source `ext/fmt/src/os.cc' not found, needed by target `build/ext/fmt/src/os.os'.

For the #1538 branch:

g++ -o build/src/extensions/pythonShim.os -c -isystem /opt/homebrew/Caskroom/miniforge/base/envs/ctdev/lib/python3.8/site-packages/numpy/core/include -isystem /opt/homebrew/Caskroom/miniforge/base/envs/ctdev/include/python3.8 -isystem include/cantera/ext -isystem /opt/homebrew/Caskroom/miniforge/base/envs/ctdev/include -std=c++17 -Wno-deprecated-declarations -O3 -Wno-inline -g -Wall -fPIC -DNDEBUG -DFMT_HEADER_ONLY=1 -DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION -Iinclude -Iinclude -Ibuild/src src/extensions/pythonShim.cpp
ext/fmt/src/format.cc:124:35: error: duplicate explicit instantiation of 'basic_data<>'
template struct FMT_API internal::basic_data<void>;
                                  ^
ext/fmt/include/fmt/format.h:739:28: note: previous explicit instantiation is here
FMT_EXTERN template struct basic_data<void>;
                           ^
1 error generated.
speth commented 12 months ago

Yes, this corresponds to changes in the organization of the {fmt} source code. Our compilation done in ext/SConscript only needs to work with the submodule version (9.1.0). This was changed in 3c9af75f0537f0378e6d0406b0b3a72753766044, if you want to swap that for local testing -- it's a pretty small change.

ischoegl commented 12 months ago

Yes, this corresponds to changes in the organization of the {fmt} source code. Our compilation done in ext/SConscript only needs to work with the submodule version (9.1.0). This was changed in 3c9af75, if you want to swap that for local testing -- it's a pretty small change.

Ah, this makes sense.