Commit 0cb890e0 authored by Mark Olesen's avatar Mark Olesen

STYLE: use findIndices instead of findStrings

- avoids needless creation of a regExg and comparison when the content
  is a plain string anyhow
parent 003a6ee0
......@@ -29,7 +29,7 @@ License
#include "IOobjectList.H"
#include "faceSet.H"
#include "demandDrivenData.H"
#include "stringListOps.H"
#include "ListOps.H"
// * * * * * * * * * * * * Private Member Functions * * * * * * * * * * * * //
......@@ -268,17 +268,15 @@ Foam::labelList Foam::Module::polyMeshGenFaces::findPatches
const word& patchName
) const
{
wordList allPatches = patchNames();
const labelList ids = findIndices(patchNames(), patchName);
labelList patchIDs = findStrings(patchName, allPatches);
if (patchIDs.empty())
if (ids.empty())
{
WarningInFunction
<< "Cannot find any patch names matching " << patchName << endl;
}
return patchIDs;
return ids;
}
......
......@@ -27,7 +27,7 @@ License
#include "pointIOField.H"
#include "IOobjectList.H"
#include "pointSet.H"
#include "stringListOps.H"
#include "ListOps.H"
// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
......@@ -89,19 +89,17 @@ Foam::labelList Foam::Module::triSurfFacets::findPatches
const word& patchName
) const
{
const wordList allPatches = patchNames();
const labelList patchIDs = findStrings(patchName, allPatches);
const labelList ids = findIndices(patchNames(), patchName);
# ifdef DEBUGtriSurf
if (patchIDs.empty())
if (ids.empty())
{
WarningInFunction
<< "Cannot find any patch names matching " << patchName << endl;
}
# endif
return patchIDs;
return ids;
}
......
  • Hello Mark,
    We have a problem here with cfMesh... This change that you made breaks the capability of cfMesh to find patches using regular expressions.

    That was the reason why I had used stringListOps.H and findStrings in the first place rather than a plain "pick out of a list" option which is what findIndices from listOps.H does.

    Could you kindly change this back to the original implementation so that regular expressions can be used again?

    Thank you.

    Regards,
    Philippose

  • mentioned in issue #6 (closed)

    Toggle commit list
  • Okay, that does make some sense, but if that is really the case we should be using either a keyType, wordRe or wordRes as the parameter. If we pass through a patch name as a word, and have it automatically convert to regex somewhere via parameter matching, this is suboptimal.

  • Hello @mark,
    Yes, I think this is something that happened during a previous OpenFOAM API change... Initially, findStrings used to directly interpret the string to be searched for as a regular expression.

    I was digging in deeper, and saw that there have been changes made to the stringListOps.H file too. What I just did a few minutes ago, was the following...:

    Foam::labelList Foam::Module::polyMeshGenFaces::findPatches
    (
        const word& patchName
    ) const
    {
        const wordRe rePatchName(patchName, wordRe::REGEX);
        
        const labelList ids = findMatchingStrings(rePatchName, patchNames());
        
        if (ids.empty())
        {
            WarningInFunction
                << "Cannot find any patch names matching " << patchName << endl;
        }
    
        return ids;
    }

    This is now working, and I can use regular expressions for the patch names.

    A similar change has to also be made in the file:

    cfmesh/meshLibrary/utilities/meshes/triSurf/triSurfFacets.C

    I was in the process of doing this when I saw your mail :-)

    What do you think of these changes?

    (Just to give you an explanation... I am doing this in real-time here, because I need OpenFOAM to work as soon as possible in order to start some simulations :-)...)

    Regards,
    Philippose

  • I've started to take a dig, but this is a bit horrible. I think it makes sense to change these to handle a keyType, since that is what it is. But then in places like meshOctreeCreatorCreateOctreeBoxes.C around line 190, we shouldn't be extracting patch names:

    const wordList patchNames = dict.toc();

    since this immediately flattens things. Instead we should have more like this:

    forAllConstIters(dict, iter)
    {
        if (!iter().isDict())
        {
            continue;
        }
    
        const keyType& pName = iter().keyword();
        const dictionary& patchDict = iter().dict();
    
    ...
    }

    That's fine, but does mean facetSubsetIndex() may or may not require a change.

  • I'd say, make the changes (probably revert) you need to get things working for you. Put it on a new branch, which we can merge in or discuss with @Juretic about his preferred direction for updating.

    I'm heading off for holiday tomorrow, so don't be insulted if you hear nothing from me until into September.

    I just added you as a developer to our cfmesh repo.

  • Thanks for giving me developer access. Shall see what can be done. @Juretic has been very quiet on the cfMesh development front, and I have not seen any changes to the original cfMesh code on sourceforge either.

    I assume "keyType" is a new data type in OpenFOAM which was added fairly recently? Had not come across this type till I started digging into 1806 earlier today. Need to go a little deeper to see what this data type is all about.

    Wish you a wonderful, enjoyable and more importantly a relaxing holiday ahead :-)!

    Regards,
    Philippose

  • Actually keyType got added with dictionary regex about 2008 (from Mattijs).

  • mentioned in commit bd2a5cc9

    Toggle commit list
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment