You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
first of all, thank you for sharing your library. Loading OpenFOAM files in Python is often cumbersome. I have written OpenFOAM field parsers in the past, and I know how challenging it can be, especially when dealing with binary data. foamlib looks like an up-and-coming tool for workflow automation, data analysis, and ML applications.
At the time of writing, I haven't installed and tested foamlib yet, so this issue concerns only a couple of thoughts I had when reading the overview article (and briefly looking through the code). @akashdhruv, @paugier, @Failxxx maybe you want to add your remarks to this issue to avoid duplication.
line 20: could you comment on why you decided to write a parser rather than create C++ bindings? Maybe a short discussion of the pros and cons of different approaches could be helpful for readers.
line 66: How is parallelism handled when creating a FoamCaseBase instance? Let's say I simulated in parallel and then reconstructed the time folders. Will TimeDirectory loop over the reconstructed or the decomposed time folders?
line 122: How is the SLURM execution configured? I couldn't find any information in the documentation or the API.
line 129: I haven't tested the code yet, but I believe it is unlikely that the parser supports the full extent of OpenFOAM syntax. For example, have you considered regular expressions, macro expansions, or include directives (overview)? These expressions are frequently encountered in all sorts of dictionaries. Maybe you have to soften fully understands slightly. In general, I think it would be sensible to include a section with unsupported file format and other features in the README or the documentation and to refer to it in the article. For example, I also noticed that the current implementation for computing the cell centers will not work for distributed meshes, which is perfectly fine. Still, to avoid upsetting users, it's better to document it somewhere.
line 131-132: I guess FoamFile does not support outputs written to postProcessing like force coefficients, probes, slices, etc. I suggest clarifying this point since users/readers might not know that not all OpenFOAM output files are FoamFiles.
table 1: in the second column of uniform field, you write np.ndarray while in other lines, you use numpy.ndarray; if possible in terms of space, I suggest using either one or the other.
Don't hesitate to ask for clarification if any of my points are unclear.
Cheers, Andre
The text was updated successfully, but these errors were encountered:
Dear @gerlero,
first of all, thank you for sharing your library. Loading OpenFOAM files in Python is often cumbersome. I have written OpenFOAM field parsers in the past, and I know how challenging it can be, especially when dealing with binary data. foamlib looks like an up-and-coming tool for workflow automation, data analysis, and ML applications.
At the time of writing, I haven't installed and tested foamlib yet, so this issue concerns only a couple of thoughts I had when reading the overview article (and briefly looking through the code). @akashdhruv, @paugier, @Failxxx maybe you want to add your remarks to this issue to avoid duplication.
FoamCaseBase
instance? Let's say I simulated in parallel and then reconstructed the time folders. WillTimeDirectory
loop over the reconstructed or the decomposed time folders?FoamFile
does not support outputs written to postProcessing like force coefficients, probes, slices, etc. I suggest clarifying this point since users/readers might not know that not all OpenFOAM output files areFoamFile
s.np.ndarray
while in other lines, you usenumpy.ndarray
; if possible in terms of space, I suggest using either one or the other.Don't hesitate to ask for clarification if any of my points are unclear.
Cheers, Andre
The text was updated successfully, but these errors were encountered: