View Issue Details

IDProjectCategoryView StatusLast Update
0001473OpenFOAM[All Projects] Bugpublic2015-01-04 15:29
ReporterwyldckatAssigned Tohenry 
PrioritylowSeverityfeatureReproducibilityN/A
Status resolvedResolutionfixed 
Product Version 
Fixed in Version 
Summary0001473: blockMesh: Provide warning when scaling option is not provided
DescriptionThe objective is to have some way of warning the user that the "convertToMeters" was misspelled, since checking for every possible typo is pointless, specially for optional settings.

This was originally reported here: http://www.cfd-online.com/Forums/openfoam-bugs/144797-blockmesh-no-spellcheck-converttometers.html
In that report, the user didn't notice a missing "t":
    converToMeters 0.001;

Since the code provides this as an option, the proposition (mostly already given on that thread) is to provide a warning when the scale wasn't provided, in file "$FOAM_SRC/mesh/blockMesh/blockMesh/blockMeshTopology.C"

Perhaps change this:

    // optional 'convertToMeters' or 'scale' scaling factor
    if (!meshDescription.readIfPresent("convertToMeters", scaleFactor_))
    {
        meshDescription.readIfPresent("scale", scaleFactor_);
    }


To this:

    // optional 'convertToMeters' or 'scale' scaling factor
    if (!meshDescription.readIfPresent("convertToMeters", scaleFactor_))
    {
        if(!meshDescription.readIfPresent("scale", scaleFactor_))
        {
            WarningIn
            (
                "blockMesh::createTopology(IOdictionary&, word&)"
            ) << "missing optional 'convertToMeters' or 'scale' scaling"
                   " factor"
                << endl;
        }
    }
TagsNo tags attached.

Activities

henry

2015-01-03 14:32

manager   ~0003475

Are you proposing that all optional inputs to OpenFOAM should be made required in case the keywoard is misspelled or only this one? If all inputs are made required complex input dictionaries like snappyHexMeshDict would become seriously cluttered! If the proposal is for only this one to be required then my question is what is so special about this one? Are there some others that are considered particulary problematic to spell correctly?

henry

2015-01-03 15:06

manager   ~0003477

It is very important that the behavior of OpenFOAM is consistent, in this case that means that if a warning is generated by readIfPresent if the convertToMeters keyword is not present it should be generated by ALL readIfPresent calls if the keyword is not present. This would generate a LARGE number of warning messages from OpenFOAM which most people would not want but this behavior could easily be put on a switch.

A better but MUCH more complex option would be for readIfPresent to check the keyword against a database of keywords scanned from the OpenFOAM sources to check for similarity with expected keywords and issue a "Did you mean..." message.

wyldckat

2015-01-03 15:31

updater   ~0003478

I think we're midway to the optimum solution here! The thought process I just went through was this:

1. The principle behind the original proposition is that there are indeed situations where the user does expect consistency from OpenFOAM, namely where he/she expects OpenFOAM to complain about everything that can be wrong. What the user might not know is that this only happens when things are strictly necessary.

2. blockMesh is the very first utility that most new users have to deal with, so it makes some sense for this utility to be more "user friendly", namely to be extra picky about every single detail :)

3. If a debug flag can be used in "readIfPresent" to warn the user on every single "didn't find this one, but will move on"... then that can be turned off on-a-need-basis. Said warning could be turned on by default and/or documented in the User Guide as "a beginner's useful warnings to turn on".

4. One thing that OpenFOAM has been lacking for sometime now is a way for the user to be aware of all options that are available from dictionaries, at least whenever possible, without having to go investigate the source code directly.


Therefore, I guess the resulting proposition at this point is something along these lines:

* Have an "if(debug)" block in "readIfPresent" (and possibly any other similar lookups) that issues a warning whenever an optional variable is not found and then indicates what will instead use the default value, since another one wasn't found.

* Have that debug flag turned off by default, but have a means to easily turn it on, for example to have a script that turns it on (hacks the "system/controlDict"?):

   foamNewbieOn

Or perhaps and additional command line option:

   -warnOptionalArguments

which will "automagically" turn on this flag and any other useful ones... which could be configured in the main "etc/controlDict", under a "newbieFeedbackDebugSwitches" block.


This seems like a good middle-ground for this, isn't it? Although the naming might not be the best, since it's the result from the idea process :)

I know that there are some debug flags that come in handy sometimes, like knowing if the mesh has some mesh triangle with -1 index or something like that, so having a quick argument option to run the solver or mesher with some useful debug flags, could be very handy indeed!

henry

2015-01-03 16:04

manager   ~0003479

Debug flags can now be set in the case controlDict so I don't think a command-line option is necessary. I can add a the optional warning to readIfPresent but I think that so many warning will be issued that the output clutter may disguise the actual issues and require users to study the output in more detail than they generally do. For the case of converToMeters it might be easier to see that the mesh wasn't scaled and check the input rather than set the readIfPresent warnings on and then wade through the output to see if converToMeters is in the list.

henry

2015-01-03 16:29

manager   ~0003480

I am working on having the writeOptionalEntries switch in InfoSwitches. I will push the change into OpenFOAM-dev for people to play with and if proved useful I will put it in OpenFOAM-2.3.x.

henry

2015-01-03 16:57

manager   ~0003481

I have just pushed this change and as expected the writeOptionalEntries switch generates a VERY large number of messages. Let me know if it useful anyway and I will make the same change to OpenFOAM-2.3.x.

wyldckat

2015-01-03 22:12

updater   ~0003488

I've now seen what you meant! It is indeed a lot of data to look at. I'll attach some example output files for future reference, from running blockMesh and icoFoam in "incompressible/icoFoam/cavity".

Honestly, this is a feature that as it is right now in this implementation, I definitely wouldn't mind having in a future version for assisting in certain debugging details.


But it looks like contextual usage would be required, so that we would be able to only get diagnostics in specific files... but that seems far from an easy implementation.
Unless the header that is read in IOobject could have an optional entry, similar to the existing "note" entry... but the problem is then propagating to the sub-dictionaries... wait, IOdictionary could establish this connection, right after checking "headerOk()"! And so could "dictionary::subDict" propagate this option from the parent to the child dictionary!


By the way, the messages for "lookupOrDefault" are incomplete:

    From function dictionary::lookupOrDefault
    in file /home/ofuser/OpenFOAM/OpenFOAM-dev/src/OpenFOAM/lnInclude/dictionaryTemplates.C at line 50
    Reading "/home/ofuser/OpenFOAM/ofuser-dev/run/tutorials/incompressible/icoFoam/cavity/system/data"
    Optional entry 'finalIteration' is not present, returning the default value '0

wyldckat

2015-01-03 22:14

updater  

log.blockMesh (52,235 bytes)

wyldckat

2015-01-03 22:16

updater  

log.icoFoam (81,817 bytes)

henry

2015-01-03 22:25

manager   ~0003489

Thanks for testing this feature, I will add the missing "'".
I will also push this feature into OpenFOAM-2.3.x.

henry

2015-01-04 15:28

manager   ~0003502

Resolved by commit 97c56ba71c141c8265d4a24455758f065491467d

Added InfoSwitches::writeOptionalEntries which enables the writing of optional keywords and values which are not present in the dictionary.

Warning: generates a VERY large number of messages from OpenFOAM applications.

Issue History

Date Modified Username Field Change
2015-01-03 11:22 wyldckat New Issue
2015-01-03 14:32 henry Note Added: 0003475
2015-01-03 15:06 henry Note Added: 0003477
2015-01-03 15:31 wyldckat Note Added: 0003478
2015-01-03 16:04 henry Note Added: 0003479
2015-01-03 16:29 henry Note Added: 0003480
2015-01-03 16:57 henry Note Added: 0003481
2015-01-03 22:12 wyldckat Note Added: 0003488
2015-01-03 22:14 wyldckat File Added: log.blockMesh
2015-01-03 22:16 wyldckat File Added: log.icoFoam
2015-01-03 22:25 henry Note Added: 0003489
2015-01-04 15:28 henry Note Added: 0003502
2015-01-04 15:28 henry Status new => resolved
2015-01-04 15:28 henry Resolution open => fixed
2015-01-04 15:28 henry Assigned To => henry