Parsing exception of phaseProperties file

[vpmapelli]

Hello, I've tried using this library to manipulate some multiphase cases, but I had some problems. I am sending the minimal case to replicate it.

System: Ubuntu 24.04
Python: 3.10.14
foamlib: 0.4.0
foamversion: OpenFOAM-9
Minimal code to replicate the error:

import os
from pathlib import Path

from foamlib._files._files import FoamFile

phase_properties = FoamFile(Path(os.environ["FOAM_TUTORIALS"]) /  'multiphase/multiphaseEulerFoam/laminar/damBreak4phase/constant/phaseProperties')

print(phase_properties["surfaceTension"])

Error message:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "~/.venv/lib/python3.10/site-packages/foamlib/_files/_files.py", line 211, in __getitem__
    _, parsed = self._read()
  File "~/.venv/lib/python3.10/site-packages/foamlib/_files/_io.py", line 61, in _read
    parsed = Parsed(self.__contents)
  File "~/.venv/lib/python3.10/site-packages/foamlib/_files/_parsing.py", line 189, in __init__
    for parse_result in _FILE.parse_string(
  File "~/.venv/lib/python3.10/site-packages/pyparsing/core.py", line 1199, in parse_string
    raise exc.with_traceback(None)
pyparsing.exceptions.ParseException: Expected end of text, found '('  (at char 633), (line:19, col:8)

It seems that some structures of this specific files has not been treated by parsing rules. By the message, it seems that this specific line is troublesome

phases (water oil mercury air);

I hope this issue helps to improve the library :)

Let me know if any further piece of information is needed. Thank you.

[gerlero]

@vpmapelli Thanks! It ended up being necessary to disambiguate between (water oil mercury air) (a list of tokens) and something like div(phi,U) (that I want to treat as a single token), but I believe #167 does it the right way.

Should be fixed in the current release (v0.4.1). Thanks again for the bug report.

[vpmapelli]

Thanks for the patch, @gerlero !

Tested again with now with version v0.4.2, but I am still getting an error with this same dictionary.

Here is the message error:

{
	"name": "ParseException",
	"message": "Expected end of text, found '('  (at char 1493), (line:79, col:1)",
	"stack": "---------------------------------------------------------------------------
ParseException                            Traceback (most recent call last)
Cell In[103], line 1
----> 1 phase_properties['phases']

File ~/.asdf/installs/python/3.10.14/lib/python3.10/site-packages/foamlib/_files/_files.py:441, in FoamFieldFile.__getitem__(self, keywords)
    438 elif not isinstance(keywords, tuple):
    439     keywords = (keywords,)
--> 441 ret = super().__getitem__(keywords)
    442 if keywords[0] == \"boundaryField\" and isinstance(ret, FoamFile.SubDict):
    443     if len(keywords) == 1:

File ~/.asdf/installs/python/3.10.14/lib/python3.10/site-packages/foamlib/_files/_files.py:211, in FoamFile.__getitem__(self, keywords)
    208 elif not isinstance(keywords, tuple):
    209     keywords = (keywords,)
--> 211 _, parsed = self._read()
    213 value = parsed[keywords]
    215 if value is ...:

File ~/.asdf/installs/python/3.10.14/lib/python3.10/site-packages/foamlib/_files/_io.py:61, in _FoamFileIO._read(self)
     58 assert self.__contents is not None
     60 if self.__parsed is None:
---> 61     parsed = Parsed(self.__contents)
     62     self.__parsed = parsed
     64 return self.__contents, deepcopy(self.__parsed)

File ~/.asdf/installs/python/3.10.14/lib/python3.10/site-packages/foamlib/_files/_parsing.py:193, in Parsed.__init__(self, contents)
    187 self._parsed: MutableMapping[
    188     Tuple[str, ...],
    189     Tuple[int, Union[FoamFileBase.Data, EllipsisType], int],
    190 ] = {}
    191 self._end = len(contents)
--> 193 for parse_result in _FILE.parse_string(
    194     contents.decode(\"latin-1\"), parse_all=True
    195 ):
    196     self._parsed.update(self._flatten_result(parse_result))

File ~/.asdf/installs/python/3.10.14/lib/python3.10/site-packages/pyparsing/core.py:1199, in ParserElement.parse_string(self, instring, parse_all, parseAll)
   1196         raise
   1197     else:
   1198         # catch and re-raise exception from here, clearing out pyparsing internal stack trace
-> 1199         raise exc.with_traceback(None)
   1200 else:
   1201     return tokens

ParseException: Expected end of text, found '('  (at char 1493), (line:79, col:1)"
}

Inspecting line number, the error occurs in the following key:

surfaceTension
(
    (air and water)
    {
        type            constant;
        sigma           0.07;
    }

    (air and oil)
    {
        type            constant;
        sigma           0.032;
    }

    (air and mercury)
    {
        type            constant;
        sigma           0.486;
    }

    (water and oil)
    {
        type            constant;
        sigma           0.03;
    }

    (water and mercury)
    {
        type            constant;
        sigma           0.415;
    }

    (oil and mercury)
    {
        type            constant;
        sigma           0.4;
    }
);

[gerlero]

Thanks! Interesting. TBH I'm not sure how foamlib should parse this:

    (water and mercury)
    {
        type            constant;
        sigma           0.415;
    }

Is this supposed to be a dict with a list as a keyword? Or should (water and mercury) be considered a keyword of type str? (I lean towards the latter...)

[vpmapelli]

I think that considering "(water and mercury)" as a keyword is more reasonable, as well. This type of syntax is used for defining behavior of pair phases, so I tend to see this as the key for the dictionary. Just to remind, there is also ordered pair configurations, with a syntax "(phase1 in phase2)", but I guess that considering them as a str key should cover this use case, as well. TBH, I would prefer if surfaceTension itself would be a dictionary, but OpenFOAM sees it as a list of pair phase configs, so, not much to do about this matter.

[gerlero]

@vpmapelli Thanks! If a str keyword is reasonable, then I'm hoping #170 fixes it.

You're welcome to test the new changes (I'll make a new release as soon as CI passes on that PR).

[gerlero]

FWIW,

from foamlib import FoamFile
FoamFile("phaseProperties").as_dict()

now outputs:

{'type': 'basicMultiphaseSystem', 'phases': ['water', 'oil', 'mercury', 'air'], 'water': {'type': 'pureIsothermalPhaseModel', 'diameterModel': 'constant', 'constantCoeffs': {'d': 0.0001}, 'residualAlpha': 0.001}, 'oil': {'type': 'pureIsothermalPhaseModel', 'diameterModel': 'constant', 'constantCoeffs': {'d': 0.0001}, 'residualAlpha': 0.001}, 'mercury': {'type': 'pureIsothermalPhaseModel', 'diameterModel': 'constant', 'constantCoeffs': {'d': 0.0001}, 'residualAlpha': 0.001}, 'air': {'type': 'pureIsothermalPhaseModel', 'diameterModel': 'constant', 'constantCoeffs': {'d': 0.003}, 'residualAlpha': 0.001}, 'blending': {'default': {'type': 'none', 'continuousPhase': 'none'}}, 'surfaceTension': [{'(air and water)': {'type': 'constant', 'sigma': 0.07}}, {'(air and oil)': {'type': 'constant', 'sigma': 0.032}}, {'(air and mercury)': {'type': 'constant', 'sigma': 0.486}}, {'(water and oil)': {'type': 'constant', 'sigma': 0.03}}, {'(water and mercury)': {'type': 'constant', 'sigma': 0.415}}, {'(oil and mercury)': {'type': 'constant', 'sigma': 0.4}}], 'interfaceCompression': [['air', 'and', 'water'], ['air', 'and', 'oil'], ['air', 'and', 'mercury'], ['water', 'and', 'oil'], ['water', 'and', 'mercury'], ['oil', 'and', 'mercury'], 1], 'aspectRatio': [], 'drag': [{'(air and water)': {'type': 'segregated', 'm': 0.1, 'n': 8}}, {'(air and oil)': {'type': 'segregated', 'm': 0.1, 'n': 8}}, {'(air and mercury)': {'type': 'segregated', 'm': 0.1, 'n': 8}}, {'(water and oil)': {'type': 'segregated', 'm': 0.1, 'n': 8}}, {'(water and mercury)': {'type': 'segregated', 'm': 0.1, 'n': 8}}, {'(oil and mercury)': {'type': 'segregated', 'm': 0.1, 'n': 8}}], 'virtualMass': [], 'heatTransfer': [], 'phaseTransfer': [], 'lift': [], 'wallLubrication': [], 'turbulentDispersion': []}

for https://github.com/OpenFOAM/OpenFOAM-9/blob/master/tutorials/multiphase/multiphaseEulerFoam/laminar/damBreak4phase/constant/phaseProperties

[gerlero]

@vpmapelli v0.4.3 is now out. I ended up having to restrict the valid characters in "(...)" keywords in order to avoid affecting the performance of the parser, but I believe it should work fine. Let me know if you have any more issues.

[vpmapelli]

It works as expected now! Thank you for this fix.

[gerlero]

FWIW, in #287 I'm experimenting with changing the way we parse these lists of keyword entries, in two ways:

1. Instead of a list of dicts, these will be parsed as lists of 2-tuples (rationale: Python dict items are 2-tuples). Any sub-dictionaries will still be parsed as dicts.
2. Instead of parsing the list-like keywords as strings, return them as actual lists (rationale: this allows removal of a special case)

Behavior similar to the previous can be recovered by doing dict(items), except that this won't work if any item keys are lists (since lists are not hashable).

@vpmapelli If you're still using this functionality and want to comment, I'm open to hearing your opinion. I'm not 100% sold on this new way of parsing these, but I'm leaning toward doing the change for v0.8.0 of foamlib.

[vpmapelli]

@gerlero , thanks for the heads up!

Those changes sound reasonable and I would say they are more consistent regarding parsing of entries between parenthesis. If I got it right, some code that would be written as:

from foamlib import FoamFile
phase_properties = FoamFile("phaseProperties").as_dict()
print(phase_properties['surfaceTension'][0]['(air and water)'])

would turn into:

from foamlib import FoamFile
phase_properties = FoamFile("phaseProperties").as_dict()
print(phase_properties['surfaceTension'][0][1])

If that is the case, changing the list-like keyword to a number would have some impact on user-code in favor of library consistency. Is this going to affect all entries of a OpenFOAM dictionary or only lists of keyword entries? As a matter of fact, this is somewhat closer to how PyFoam parses these kind of entries, but I would prefer this kind of parsing, as PyFoam does not create a list of 2-tuples, but a single list of len(2n), where i-th element is a list em 2i-th a dictionary.

From the point of view of code layering, I can undestand that replicating PhasePair hashing from OpenFOAM should be implemented by user code from the list parsed form the list-like keyword. But, for my use case, for example, just for some fast case manipulations, I'd say it would not have that much impact.

To be fair, I am in no place to give my opinion, since I used this library in an early version for manipulating some simple cases. I am still monitoring, though, some libraries to handle OpenFOAM cases and haven't adopted one definetly yet. But, personally, I actually still prefer the list of dicts parsing as I cannot see a strong positive benefit of this change. But I guess that as a user, I am probably missing something such better consistency on library code or performance-wise issues.

[gerlero]

Those changes sound reasonable and I would say they are more consistent regarding parsing of entries between parenthesis. If I got it right, some code that would be written as:

from foamlib import FoamFile
phase_properties = FoamFile("phaseProperties").as_dict()
print(phase_properties['surfaceTension'][0]['(air and water)'])

would turn into:

from foamlib import FoamFile
phase_properties = FoamFile("phaseProperties").as_dict()
print(phase_properties['surfaceTension'][0][1])

If that is the case, changing the list-like keyword to a number would have some impact on user-code in favor of library consistency.

That's right!

Is this going to affect all entries of a OpenFOAM dictionary or only lists of keyword entries?

Only lists of keyword entries.

As a matter of fact, this is somewhat closer to how PyFoam parses these kind of entries, but I would prefer this kind of parsing, as PyFoam does not create a list of 2-tuples, but a single list of len(2n), where i-th element is a list em 2i-th a dictionary.

Yeah, I don't like how PyFoam does it either, so I wouldn't go that way. Honestly, I believe PyFoam's way is an accident of how they parse their list entries and not really carefully thought out for this scenario.

From the point of view of code layering, I can undestand that replicating PhasePair hashing from OpenFOAM should be implemented by user code from the list parsed form the list-like keyword. But, for my use case, for example, just for some fast case manipulations, I'd say it would not have that much impact.

To be fair, I am in no place to give my opinion, since I used this library in an early version for manipulating some simple cases. I am still monitoring, though, some libraries to handle OpenFOAM cases and haven't adopted one definetly yet. But, personally, I actually still prefer the list of dicts parsing as I cannot see a strong positive benefit of this change. But I guess that as a user, I am probably missing something such better consistency on library code or performance-wise issues.

Anyway, thanks for chiming in on this one! I'll consider your opinion no matter which way I choose to go.