View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002396 | OpenFOAM | [All Projects] Bug | public | 2016-12-15 12:41 | 2018-04-20 21:27 |
| Reporter | MattijsJ | Assigned To | henry | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Platform | GNU/Linux | OS | OpenSuSE | OS Version | 13.2 |
| Product Version | dev | ||||
| Fixed in Version | dev | ||||
| Summary | 0002396: dictionary scoped look up too forgiving? | ||||
| Description | Following dictionary: someDict { subDict { myKey myValue; } } geometry { $:someDict2.subDict; // note: someDict missspelled } in v3/v4 it complains that there is no 'someDict2' entry at the top level in dev it does not complain but just takes it for a non-existing entry called 'someDict2.subDict'. This is because in dev we extended the lookup at any level to include words with a '.' (to e.g. handle key 'motorBike.stl'). Hmm. Any suggestions? | ||||
| Tags | No tags attached. | ||||
|
|
I didn't take a proper look at the implementation that was done, but I had gotten the impression that names with dots were only valid if they were in quotes. The other possibility is to go the same way that regular expressions do their job: a dot is a dot only when escaped with a backslash, e.g.: $:someDict.subDict $:someDict.motorbike\.stl |
|
|
Quotes makes dictionary entries wildcards which are stored completely separate and have separate matching rules. Backslash: it is pretty exceptional that we're trying to use '.' in a word used for looking up things in a dictionary so a special interpretation of the keyword is not a big problem. Attached dictionary.C that implements it: foamDictionary ./snappyHexMeshDict -entry 'geometry.motorBike\.obj' Feel free to test & break it. dictionary.C (27,670 bytes) |
|
|
With that version recursive substitution still works as long as the word includes the '\': motorBike.obj 10; var motorBike\.obj; b ${$var}; produces b 10; |
|
|
The real problem is that motorBike.obj should never be a keyword. Syntax in snappyHexMeshDict, surfaceFeatureExtractDict, that uses the filename as a keyword is inconsistent. motorBike.obj { type triSurfaceMesh; name motorBike; } should be motorBike { type triSurfaceMesh; name motorBike.obj; // or filename motorBike.obj } The filename should not be one level higher than the type because the need for a filename is a consequence of the type being triSurfaceMesh. Same with surfaceFeatureExtract, where the consequences for the user are much worse because of the need to have a separate dictionary for every surface. motorBike.obj { extractionMethod extractFromSurface; ... Here, the dictionary name could be the method, and the dictionary include an entry for all surfaces, e.g. extractFromSurface { surfaces (motorBike.obj anotherSurface.obj); ... |
|
|
triSurfaceMesh.H (9,113 bytes) |
|
|
Attached triSurfaceMesh[CH] to accept motorBike { type triSurfaceMesh; fileName "motorBike.obj"; } triSurfaceMesh-2.H (9,113 bytes) |
|
|
triSurfaceMesh.C triSurfaceMesh.C (20,840 bytes) |
|
|
Isn't the problem that a '.' is a valid word character and thus automatically a valid character for a keyword, but the '.' has also been double-purposed as a scoping character? IMO a consistent solution would be to have '/' as the scoping character, which is definitely not a valid word character and require surrounding scoped variables with ${ }. |
|
|
Allowing "." as a valid keyword character was arguably not ideal, but we probably need to live with it. As far as I can tell, it is only included as a character in keywords that are: 1) filenames, e.g. motorBike.obj 2) fields of particular phases, e.g. alpha.water, T.steam Using filenames as keywords is bad design. I suggest that is eliminated throughout the code. That leaves only fields of phases as the one exception, which I propose we handle with "\." unless that causes some kind of critical failure. PS Surely it is too late for "/" as the scoping character? which was considered, but rejected as a confusing scoping character. I cannot recall whether using "_" for fields of phases, e.g. alpha_water, was only rejected because of p_rgh, but I guess that would have been better. |
|
|
I've thought about this as well and forgot to write down sooner what I've remembered/associated so far, also because I still wanted to double-check some of these possibilities. Either way, before it's too late, adding to what Chris has already listed, here are the additional situations that may need to be kept in mind: - Using slash '/' as a scoping character could potentially lead to problems when using '#calc', although I haven't tested yet if it even supports these scoped variables; but I guess that using '${}' would have solved that issue, as Mark mentioned. - There is a particular type of object/file naming convention which may also interfere with the parsing of the scopes, for example, names such as 'thermo:psi' and 'cloudname:UCoeff'. I haven't checked how these are parsed/interpreted, but could potentially conflict with the colon as a prefix (e.g. '$:thermo:psi.boundaryField.inlet.value'). - Using the new convention to have file names explicitly defined through a keyword, is a big advantage for people who work a lot with CAD and then have file names like "30+-0.1mm washer.stl", which would be a nightmare when trying to mesh a few hundred or more of them with 'snappyHexMesh'. |
|
|
Update on my comment " Using filenames as keywords is bad design. I suggest that is eliminated throughout the code." The use in snappyHexDict (motorBike.obj...) was fixed by the following commit: https://github.com/OpenFOAM/OpenFOAM-dev/commit/80e22788e4c6fe0dbdfe2cb5004eefc0fb49c121 The use in surfaceFeatureExtract has not been fixed, as suggested: motorBike.obj { extractionMethod extractFromSurface; ... should be replaced with extractFromSurface { surfaces (motorBike.obj anotherSurface.obj); |
|
|
I believe that with the recent commit aaed6290d06976caf4c878a41734d5b9a0c50d3c - https://github.com/OpenFOAM/OpenFOAM-dev/commit/aaed6290d06976caf4c878a41734d5b9a0c50d3c - this report can now be marked as resolved, given that 'surfaceFeatureExtract' can be discontinued in the (near) future, now that the new 'surfaceFeatures' utility as come to existence. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2016-12-15 12:41 | MattijsJ | New Issue | |
| 2016-12-15 12:46 | wyldckat | Note Added: 0007487 | |
| 2016-12-15 18:02 | MattijsJ | File Added: dictionary.C | |
| 2016-12-15 18:02 | MattijsJ | Note Added: 0007489 | |
| 2016-12-16 10:05 | MattijsJ | Note Added: 0007497 | |
| 2016-12-16 10:21 | chris | Note Added: 0007500 | |
| 2016-12-16 10:55 | henry | Note Edited: 0007500 | View Revisions |
| 2016-12-20 16:00 | MattijsJ | File Added: triSurfaceMesh.H | |
| 2016-12-20 16:01 | MattijsJ | File Added: triSurfaceMesh-2.H | |
| 2016-12-20 16:01 | MattijsJ | Note Added: 0007538 | |
| 2016-12-20 16:01 | MattijsJ | File Added: triSurfaceMesh.C | |
| 2016-12-20 16:01 | MattijsJ | Note Added: 0007539 | |
| 2016-12-20 21:01 |
|
Note Added: 0007540 | |
| 2017-01-05 17:20 | chris | Note Added: 0007598 | |
| 2017-01-05 23:19 | wyldckat | Note Added: 0007600 | |
| 2017-06-30 12:32 | chris | Note Added: 0008306 | |
| 2017-06-30 13:22 | chris | Assigned To | => chris |
| 2017-06-30 13:22 | chris | Status | new => feedback |
| 2017-06-30 13:23 | chris | Assigned To | chris => |
| 2017-06-30 13:23 | chris | Status | feedback => assigned |
| 2017-06-30 13:24 | chris | Assigned To | => chris |
| 2017-06-30 13:24 | chris | Status | assigned => new |
| 2017-06-30 13:25 | chris | Assigned To | chris => |
| 2018-04-20 21:26 | wyldckat | Note Added: 0009510 | |
| 2018-04-20 21:27 | wyldckat | Assigned To | => henry |
| 2018-04-20 21:27 | wyldckat | Status | new => resolved |
| 2018-04-20 21:27 | wyldckat | Resolution | open => fixed |
| 2018-04-20 21:27 | wyldckat | Fixed in Version | => dev |
