View Issue Details

IDProjectCategoryView StatusLast Update
0002353OpenFOAM[All Projects] Bugpublic2016-12-09 23:15
ReporterjoegiAssigned Tohenry 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
PlatformGNU/LinuxOSOpenSuSEOS Version13.2
Product Version4.x 
Fixed in Versiondev 
Summary0002353: yplus functionobject does not show the value on screen
DescriptionIt seems that this issue is not solved yet. I updated my OF4.x installation and I still have this issue.

I would expect a behavior similar to the one in version 3.x, where you have the values of yplus in the standard output, but in this version I just get the yplus value when the field is saved.

This is the FO I am using (which works ok in version 3.x),

yplus
{
    type yPlus;
    functionObjectLibs ("libutilityFunctionObjects.so");
    enabled true;

    //outputControl outputTime; //of3x
    writeControl outputTime; //of4x

    outputControl timeStep;
    outputInterval 10;

    log true;
}
TagsNo tags attached.

Relationships

related to 0002320 resolvedhenry yplus functionobject does not show the value on screen 
related to 0002355 resolvedhenry Main function object description is missing "log" and has mislabelled "evaluate" 

Activities

wyldckat

2016-11-26 17:19

updater   ~0007332

I've got a preliminary evaluation of the current state of the yPlus function object:

1. The updated configuration for your example to work properly on OpenFOAM 4.x is this:

    yplus
    {
        type yPlus;
        functionObjectLibs ("libfieldFunctionObjects.so");
        enabled true;

        writeControl timeStep;

        executeControl timeStep;
        executeInterval 10;

        log true;
    }

  a. There is a typo in the description of the function objects, which I'll report separately, namely that "execute*" is not properly documented.


2. The data logged to the output is currently only done when writing, for efficiency reasons, because this is when the files are all written and it's when the min-max values are calculated.


3. There is an issue that is related to this, which I've seen reported on the OpenFOAM-plus issue tracker: if the execute and write controls don't match up, the results saved to file will be stale.


4. The write method for this yPlus function object is looking a bit strange, mostly because there are two write steps:

      yPlus.write();

      writeFiles::write();

   I still haven't fully diagnosed this and it looks a bit strange... it does seem to be because yPlus is created with AUTO_WRITE and it does make some sense that we should make sure it's written, but it still looks a bit strange.


Let me first sort out the the 1.a issue and to think a bit better about this... because at first glance, it seems that we would need several time controls, or at least two entries for logging: executeLog, writeLog... which sounds fairly redundant.

henry

2016-11-26 17:35

manager   ~0007333

> yPlus is created with AUTO_WRITE

I have just fixed this in OpenFOAM-dev, I will do the same for OpenFOAM-4.x

joegi

2016-11-26 17:49

reporter   ~0007334

Ok, let's see if you manage to shape up this FO.

But why don't you fall back to the same behavior as in OF3, that is, getting the yplus values on the standard output in every iteration without the need of saving the fields every single iteration?

By the way, when using combinations of FO from time to time they stale (as you mentioned and as I reported previously), I didn't have this problem in previous versions.

I know, when I do production runs I try to avoid FO or crazy combinaitons of FO, but during preliminary runs I use many of them to monitor the behavior of the the solution, and many times the simulation just stale for a while.

wyldckat

2016-11-26 18:11

updater   ~0007335

> and many times the simulation just [stalls] for a while.

That would need a really good test case in order to reproduce the same issue.


> But why don't you fall back to the same behavior as in OF3, that is, getting the yplus values on the standard output in every iteration without the need of saving the fields every single iteration?

I do like to keep things running at maximum performance... so I do like the change, although the functionality for getting min-max yPlus values while running is also a must for debugging the runs on-the-fly... so it does feel like we should have something that could give us the ability to take a peak... wait... we do have a function object for that...

wyldckat

2016-11-26 18:27

updater   ~0007337

OK, so the reason why yPlus was using 'writeFiles' in 4.x was because it was going to be replaced later on in dev for 'logFiles'...

wyldckat

2016-11-26 19:11

updater   ~0007338

I was going to say/write that the 'fieldMinMax' function object could be used to reproduce at least part of the missing feature, but it can't because it doesn't allow having the calculating being made per patch.


@joegi:
> But why don't you fall back to the same behavior as in OF3

Several utilities were converted/merged into function objects, to remove duplicate code and make more general-purpose, i.e. the current feature set allows for most or all solvers to do the same post-processing, while in OpenFOAM 3.0.x it was only possible for the more common transport models (single phase, (in)compressible flow) and others were left out (multiphase and so on).

This to say that the current yPlus function object reproduces the respective old utility in 3.0.x (min/max/avg only on write) and still does mostly the same as the old function object (e.g. any transport model).


But hold on, are you OK with the fact that in OpenFOAM 3.0.x all min/max/avg values were written every 10 time steps along with the yPlus calculation? Because they are shown in the standard output and also in the respective log file!

wyldckat

2016-11-26 19:50

updater   ~0007339

So here's the problem and respective options I can think of:

1. The objective for the change made from 3.0.* to 4.* was essentially to consolidate features and make the said functionality consistent. In this case: the 'write' phase was made more explicitly consistent so that it writes anything that needs writing during this phase.

2. The undesired side-effect is that writing the 'yPlus' field at the same time as the min/max/avg values are logged is counter-productive (regardless of standard output or not). This is because many times we only want to keep track of how things are progressing and we don't need the field itself written to disk, or vice-versa (no min/max needed).

3. Relying on "AUTO_WRITE" was as step in a good direction towards making it easier to only write yPlus when all others are written... but this would lead to the issue of having the field go stale...

4. Furthermore, yPlus, wallShearStress, wallHeatFlux are all in the same boat... the 3 need one or more patch operations of min, max, avg and sum/integral values.

5. Therefore, from the perspective of generalization, we should extend the current 'fieldMinMax' and 'fieldAverage' (and later on other field* function objects) to also support per-patch calculations and therefore drop the current min/max/avg feature from the 'yPlus', 'wallShearStress', 'wallHeatFlux' function objects. This would also allow for a bit more refined control on when this should be shown on standard output and/or logged to disk.

6. Fixing this right away in a single fix isn't straight forward:

  - In 4.x we shouldn't make such a big change, because it would break the existing feature set (only write when things are written and only one function object does 2 operations).

  - In dev it will take some considerable time to implement the by-patch min/max/.. feature, although it's conceptually similar to what's already in 'wallShearStress', 'wallHeatFlux', namely the use of 'patchSet'.



@Henry: Would adding a temporary option 'writeField' (defaulted to 'yes') to 'yPlus' and 'wallShearStress' function objects in 4.x be acceptable? Because:

 - this would allow to rely on the 'writeObjects' function object to get the field written when needed and still get the old functionality working.

 - and it's a relatively small hack and it would gives us some more time to implement #5 (generalization of min/max/avg/sum per patch and subsequent logging on stdout and files).

henry

2016-11-26 20:07

manager   ~0007341

Why do you need a special option in OpenFOAM-4.x? The 'yPlus' field is registered and can be written using the 'writeObjects' functionObject or is this not working in OpenFOAM-4.x?.

If a new option is needed in OpenFOAM-4.x why is it not needed in OpenFOAM-dev and future releases?

Also if it is needed for 'yPlus' and 'wallShearStress' why not for the other functionObjects? What is the advantage of merging the 'writeObjects' functionality into other functionObjects? If this is needed then a functionObject base-class would be needed to provide IO support for fields created and registered by those functionObjects.

henry

2016-11-26 20:12

manager   ~0007342

Adding patch-based functionality to 'fieldMinMax' would not break backward-compatibility and hence could be added to OpenFOAM-4.x if needed.

wyldckat

2016-11-26 20:32

updater   ~0007343

> The 'yPlus' field is registered and can be written using the 'writeObjects' functionObject or is this not working in OpenFOAM-4.x?.

Yes, it's working... at least last time I tried using it, it was still working...

The problem is that when "functionObjects::yPlus::write()" is called, it's writing both the 'yPlus' and the log file, which isn't what users always need, because there are a lot of situations where only the log file is needed (e.g. all iterations).

For example, as reported in this issue, in 3.0.x the min/max/avg values could be logged every time step, while the yPlus field was always written.


> If a new option is needed in OpenFOAM-4.x why is it not needed in OpenFOAM-dev and future releases?
> Also if it is needed for 'yPlus' and 'wallShearStress' why not for the other functionObjects?
> If this is needed then a functionObject base-class would be needed to provide IO support for fields created and registered by those functionObjects.

Mmm... I was aiming towards in dev and future versions to rely on other function objects to do the logging separately of min/max/average/sum values for patches.

Although... I see what you mean. The fields could be registered into the base-class and written from there if so chosen. This is fairly straight forward to implement and would solve the current issue.


> What is the advantage of merging the 'writeObjects' functionality into other functionObjects?

No, sorry, the idea is to use it separately. The end-user can rely on 'writeObjects' to write the fields later on, while it's disabled e.g. in the 'yPlus' function object.


> Adding patch-based functionality to 'fieldMinMax' would not break backward-compatibility and hence could be added to OpenFOAM-4.x if needed.

I am expecting a major change inside the inner workings of 'fieldMinMax', which was why I was thinking of keeping it out from 4.x... that and it will probably take a considerable amount of time to implement and test.



OK, base-class for optionally writing fields it is!

henry

2016-11-26 20:50

manager   ~0007345

> The problem is that when "functionObjects::yPlus::write()" is called, it's writing both the 'yPlus' and the log file, which isn't what users always need, because there are a lot of situations where only the log file is needed (e.g. all iterations).

Fair point. The easiest solution would be to remove/switch the writing of 'yPlus' from the write function and allow the user to control the writing separately using the 'writeObjects' functionObject but this is rather inconvenient for common usage and separates the writing from when the field is updated. It would probably need a 'writeField' switch so that the current behaviour can still be used as it probably the most common usage.

The problem with the base-class idea is that it would not know how many fields need to be controlled in this manner so could only provide the logic and controls for writing fields and this used in the derived classes which hold the fields and call the 'write'. What if there are several fields and the user wants to control the output of each separately? Then it would be easier to use several 'writeObjects' functionObjects to control the writing separately.

> I am expecting a major change inside the inner workings of 'fieldMinMax',

Sure but if the interface is backward-compatible it could go into OpenFOAM-4.x

> that and it will probably take a considerable amount of time to implement and test.

Indeed it will.

wyldckat

2016-11-27 23:21

updater   ~0007347

Things didn't go as I planned for today, so I'm attaching a quick hack for @joegi and anyone else who is looking for this feature. This should only be considered as a quick proof of concept and workaround for the reported issue. A proper fix will take a few more hours to get it done properly.

The attached package "yPlus.4x.writeFieldHack.tar.gz" provides a replacement for the files at "src/functionObjects/field/yPlus/" in OpenFOAM 4.x, where "writeField" boolean option is provided for this function object.

To use it, here is the quick update for one of the examples above:

    yplus
    {
        type yPlus;
        functionObjectLibs ("libfieldFunctionObjects.so");
        enabled true;

        writeControl timeStep;
        writeInterval 10;
        writeField false;

        executeControl timeStep;
        executeInterval 10;

        log true;
    }

Keep in mind that 'writeObjects' still has to be used to write 'yPlus' at output time only.


@Henry: Many thanks for the detailed feedback on this! I'm with you on most of the details, but there is one particular detail that you mentioned that reveals an existing issue:

> and separates the writing from when the field is updated.

Unfortunately this already happens. Currently there is no option in place to ensure the field is updated for writing, e.g. in the 'yPlus' function object; this is currently left to the end user to make sure when it's being calculated and when it's being written, matches the needs of the user.

yPlus.4x.writeFieldHack.tar.gz (2,685 bytes)

wyldckat

2016-12-04 11:14

updater   ~0007391

@Henry: I've attached the package 'proposition_dev.v1.tar.gz' which provides the following files and respective changes for OpenFOAM-dev:

 - src/OpenFOAM/Make/files

   - Adds the source file below, namely 'writeFields.C'.

 - src/OpenFOAM/db/functionObjects/writeFields/writeFields.C
 - src/OpenFOAM/db/functionObjects/writeFields/writeFields.H

   - This is a new base class which is conceptually similar to 'logFiles', but is aimed at managing fields that are meant to be written by the function object that inherits from this class.

   - It has also been structured for 'writeObjects' to inherit from this class, along with overriding some of the virtual methods for extending the intended features, specifically the methods 'read' and 'writeObject'.

   - It's still missing a bit more description in the header on how to use it.

   - This also requires that all function objects that inherit this class, must create the fields as registered objects, like 'yPlus' does. This can be useful for chaining function objects to do several calculations and analysis.

 - src/functionObjects/field/yPlus/yPlus.C
 - src/functionObjects/field/yPlus/yPlus.H

   - Has been modified to inherit 'writeFields' and use it accordingly, as a proof of concept.


This is proposed for intermediate review because there are a few details I'm still divided on:

 1. The name 'writeFields' sounds correct in the context of being used by 'yPlus' and other similar function objects, but gets a bit confusing because the fields are managed as objects. Furthermore, the design is such that 'writeObjects' should inherit from this class, which adds a bit more confusion name-wise. The other name I can see as appropriate is 'writeLocalObjects', which also sounds a bit strange, even though it's a lot more accurate.

 2. The added 'writeFields' keyword for the dictionaries is a 'wordReList' as 'objects' from 'writeObjects', which also raises the question: Should 'objects' be used as the default keyword as well for this new base class? And if so, should it also have the keyword backward compatibility that 'writeObjects' has got?

 3. Should a keyword switch be added for making it easier to disable writing all fields? Because with this proposition, we can disable writing all fields like this:

    writeFields ();

   which seems a bit of a strange way to say "no" or "false".

proposition_dev.v1.tar.gz (8,047 bytes)

henry

2016-12-04 11:23

manager   ~0007392

Do you see any problem deriving 'writeObjects' from 'writeFields'? Would it be appropriate to combine the functionality and derive 'yPlus' from the new 'writeObjects' or does it provide functionality that 'yPlus' should not have?

Another possible naming convension is to name 'writeFields' -> 'writeObjectsBase' because it should be used only as a base-class and not present on the runtime selection table.

wyldckat

2016-12-04 13:37

updater   ~0007393

> Do you see any problem deriving 'writeObjects' from 'writeFields'?

Only regarding the names, that's all.


> Another possible naming convention is to name 'writeFields' -> 'writeObjectsBase' because it should be used only as a base-class and not present on the runtime selection table.

That name does sound better.


> Would it be appropriate to combine the functionality and derive 'yPlus' from the new 'writeObjects' or does it provide functionality that 'yPlus' should not have?

I'm not sure if I understood you correctly... did you mean 'writeObjects' or 'writeFields'? I ask this because I can see the following possible interpretations/answers:

 - The proposed 'yPlus' already derives from the proposed 'writeFields' as an add-on, the same way as 'logFiles', so I guess this wasn't what you were asking about.

 - Having 'yPlus' derive directly from 'writeObjectsBase' (i.e. 'writeFields') instead of 'fvMeshFunctionObject', would be a bit confusing the sense of the naming convention. But it would simplify internal code in the new 'writeObjectsBase' class, namely not needing to rely on local const references to 'log' and 'obr'.

 - Having 'yPlus' derive from the current 'writeObjects' would be overkill and would lead to scope issues, i.e. local vs global; along with problems associated to having the 'writeOption' available to 'yPlus', which would lead to confusion and misuse.


----

On the other hand, if instead we had 'regionFunctionObject' derive from 'writeObjectsBase'... would possibly make the most sense. I only noticed just now that 'regionFunctionObject' already has some registered object handling, so splitting it in two, where 'regionFunctionObject' would limit the scope to the current region, while 'writeObjectsBase' would limit to an optional scope of the local names...

Now this raises the question: If 'regionFunctionObject' has several convenient object handling methods, shouldn't 'removeRegisteredObject' and 'writeObjects' derive from it instead? Because 'removeRegisteredObject' is also requesting for a 'region' keyword/scope...


OK, so one of the conclusions I ended up at is that we should have 'writeObjectsBase' derive from 'regionFunctionObject' with the 'virtual' prefix, so that it can be overloaded by the classes that need a more refined object scope management for writing fields. If I'm not mistaken, it would be something like the following for 'writeObjectsBase':

    class writeObjectsBase
    :
        public virtual regionFunctionObject

This would allow us to reuse the object-management methods in 'regionFunctionObject'... although the same would be needed in 'fvMeshFunctionObject':

    class fvMeshFunctionObject
    :
        public virtual regionFunctionObject

henry

2016-12-04 15:13

manager   ~0007394

> I'm not sure if I understood you correctly... did you mean 'writeObjects' or
'writeFields'?

If 'writeFields' and 'writeObjects' are combined into 'writeObjects' then there is only 'writeObjects' to derive 'yPlus' from.

> - Having 'yPlus' derive directly from 'writeObjectsBase' (i.e. 'writeFields') instead of 'fvMeshFunctionObject', would be a bit confusing

My understanding is that you have alreadly derived 'yPlus' from 'writeFields' (to be renamed 'writeObjectsBase' or merged into 'writeObjects'), how is this confusing and what do you propose to avoid the confusion?

> - Having 'yPlus' derive from the current 'writeObjects' would be overkill and would lead to scope issues, i.e. local vs global; along with problems associated to having the 'writeOption' available to 'yPlus', which would lead to confusion and misuse.

OK.

> On the other hand, if instead we had 'regionFunctionObject'

functionObjects derived from 'regionFunctionObject' do not necessarily write fields but we could create an alternate intermediate class derived from 'regionFunctionObject' which adds the object writing functionality which looks like what you are proposing 'writeObjectsBase' to be.

virtual derivation is best avoided and I do not see a need for it in this context.

henry

2016-12-04 15:26

manager   ~0007395

The other option is multiple-inheritance, deriving 'yPlus' from fvMeshFunctionObject, logFiles and writeObjectsBase which provids the field writing functionality rather than itself being a functionObject.

wyldckat

2016-12-04 16:06

updater   ~0007396

> virtual derivation is best avoided and I do not see a need for it in this context.

Makes sense... it would be hard to keep track on which one is which and in what order.


> The other option is multiple-inheritance, deriving 'yPlus' from fvMeshFunctionObject, logFiles and writeObjectsBase which provids the field writing functionality rather than itself being a functionObject.

That is currently in the first proposition, although with the 'writeFields' name.


I'm already trying out implementing the intermediate option:

    class writeObjectsBase
    :
        public regionFunctionObject

and

    class fvMeshFunctionObject
    :
        public writeObjectsBase


It has the advantage of keeping the class 'writeObjectsBase' much simpler and be usable by 'writeObjects'.

henry

2016-12-04 16:21

manager   ~0007397

fvMeshFunctionObjects do not necessarily write fields and hence should not be derived from 'writeObjectsBase'. For flexiblity I think it would be best to use the multiple-inheritance option.

> It has the advantage of keeping the class 'writeObjectsBase' much simpler and be usable by 'writeObjects'.

In what way is 'writeObjectsBase' simpler and why is it not possble to use it for 'writeObjects' if it not itself a functionObject?

wyldckat

2016-12-04 16:45

updater   ~0007398

>> It has the advantage of keeping the class 'writeObjectsBase' much simpler and be usable by 'writeObjects'.

> In what way is 'writeObjectsBase' simpler and why is it not possble to use it for 'writeObjects' if it not itself a functionObject?

Sorry, I should have written "and continue to be usable by 'writeObjects'"...

In review:

 - If 'writeObjectsBase' is used for multiple-inheritance, it requires const reference access to the variables 'log', 'obr', 'typeName' and 'name' from the derived function object, namely for logging output when required and for handling the objects.

 - If 'writeObjectsBase' inherits directly from 'regionFunctionObject', it doesn't need to replicate code for writing objects. And inheriting the region selection would also simplify a bit more 'writeObjects'.


Other alternatives:

 - 'writeObjectsBase' could not override the 'read(dict)' and 'write()' methods and be called from the derived classes from 'read' and 'write' on a need basis. It would have the downside of having a few more methods and variables.

 - Yet another possibility is to have 'fvMeshFunctionObject' as a templated class, therefore only having inheritance on a need basis, deriving from 'regionFunctionObject' or 'writeObjectsBase'... which does have other advantages, in case a 3rd or 4th options ever come up.

This last one seems to be the most elegant solution so far, or at least heading in the right direction!?

henry

2016-12-04 17:07

manager   ~0007399

For functionObjects which log information and optionally write fields it is probably not a good idea to override the read and write methods because the appropriate methods need to be called for both the logging and the field writing. If the 'writeObjectsBase' holds a reference to the functionObject it is associated with all the information it needs would be available and the read and write functions called from the derived class when necessary. I have not yet studied the code in details so maybe this is not as good an approach as derivation but it seems the most flexible.

I would rather avoid templating in this virtual hierachy if possible; I put in quite a bit of effort to remove the unnecssary templating used in the original implementaion which added an insane amount of complexity.

wyldckat

2016-12-04 17:27

updater   ~0007400

OK, understood and will do!

wyldckat

2016-12-04 22:57

updater   ~0007401

Henry, many thanks for the various quick feedbacks!

Hopefully the attached package "proposition_dev.v2.tar.gz" for OpenFOAM-dev does exactly what has been discussed here and the 'writeObjectsBase' is actually fairly simple now and didn't need as many reference variables as I initially thought it did, nor does need to derive from any other class.

This proposition provides the following files and changes:

 - src/OpenFOAM/Make/files

   - Adds the source file below, namely 'writeObjectsBase.C'.

 - src/OpenFOAM/db/functionObjects/writeObjectsBase/writeObjectsBase.C
 - src/OpenFOAM/db/functionObjects/writeObjectsBase/writeObjectsBase.H

   - This is a new base class which is conceptually similar to 'logFiles', but is aimed at managing fields that are meant to be written by the function object that inherits from this class.

   - It has also been structured for 'writeObjects' to inherit from this class, along with overriding some of the virtual methods for extending the intended features, specifically the methods 'read', 'writeObject' and 'objectNames'.

   - It's now fully documented in the description section of the header.

   - This enables function objects that inherit this class to define through the methods 'resetLocalObjectName' or 'resetLocalObjectNames' the fields (or objects) that are or will be registered objects, like 'yPlus' does. Bonus is that having those fields registered can be useful for chaining function objects to do several calculations and analysis.

   - The structure is not strict and is flexible in such as way that can be used differently, as detailed for the following classes.

   - The keyword for the dictionary is now "objects" instead of "writeFields", to make it consistent with the nomenclature stipulated by 'writeObjects'.

   - The lookup of objects does not drop the const restraint, in opposition to what's in the current 'writeObjects', which is a benefit to avoid accidental changes to the object.

 - src/functionObjects/field/yPlus/yPlus.C
 - src/functionObjects/field/yPlus/yPlus.H

   - This class now also inherits 'writeObjectsBase' and assigns the field 'yPlus' to be written by 'writeObjectsBase'.

   - Special care was put into the design of 'writeObjectsBase' and 'yPlus', so that when 'log true;' it will give the same output, along with a line break at the end of the function object's 'write' method for consistency.

   - The changes done to this class were fairly minimal.

 - src/functionObjects/utilities/writeObjects/writeObjects.C
 - src/functionObjects/utilities/writeObjects/writeObjects.H

   - This class now also inherits from 'writeObjectsBase', along with overriding the methods 'objectNames', 'writeObject' and 'read' from that inherited class.

   - The contents of the method 'write' have been split across three methods:

     - 'writeObjectsBase::write' does the 'forAll' loop and object lookup, after calling the method 'objectNames' for the loop;

     - 'writeObjects::objectNames' does the dynamic gathering of the object names;

     - 'writeObjects::writeObject' does the decision on whether the object should be written or not, depending on 'writeOption'.

   - Notes:

     - The variable 'writeObjectsBase::localObjectNames_' is not used by 'writeObjects', because it doesn't need to.

     - 'obr_' and 'objectNames_' have been transferred from 'writeObjects' to 'writeObjectsBase', respectively renamed to 'writeObr_' and 'writeObjectNames_'.



The example for not writing the 'yPlus' field and then writing it later through 'writeObjects' is for this proposition:

    yplus
    {
        type yPlus;
        functionObjectLibs ("libfieldFunctionObjects.so");
        enabled true;

        writeControl timeStep;
        writeInterval 10;
        objects ();

        executeControl timeStep;
        executeInterval 10;

        log true;
    }

    writeYPlus
    {
        type writeObjects;
        libs ("libutilityFunctionObjects.so");
        writeControl outputTime;
        objects (yPlus);
        log true;
        writeOption anyWrite;
    }



If this second proposition is accepted, we can move forward with re-adapting this feature to the other function objects that gain from it.

wyldckat

2016-12-04 22:58

updater  

proposition_dev.v2.tar.gz (9,738 bytes)

henry

2016-12-06 12:42

manager   ~0007415

This proposal looks good. Just one question:

        //- Object names that are handled on behalf of the inheritor
        wordList localObjectNames_;

        //- Object names requested by the user to be written
        wordReList writeObjectNames_;

Why separate the list of object names? Why not insert the 'local' object names directly into 'writeObjectNames_'? This would remove a lot of clutter.

wyldckat

2016-12-06 13:03

updater   ~0007416

The two lists are needed because:

 - 'localObjectNames_' has the names of the fields that the function object has registered and will be using.

 - 'writeObjectNames_' is a regular expression list, for convenience, which is applied only to the list in 'localObjectNames_'.

Having only one list would be dangerous, because it would allow to handle any registered IO object, not only the ones created by this function object.

Unless there is some kind of double-headed list I'm not aware of, similar to a hash list... although... we could use a single hash list with a word-bool pair and have 'read(dict)' process that information into that single hash list, therefore discarding the 'objectNames' method. The downside of that is if the function object updates the local list of objects midway, it would have to call 'read(dict)' again.

henry

2016-12-06 13:30

manager   ~0007417

If the names of the fields to write are in 'localObjectNames_' why is 'writeObjectNames_' needed?

henry

2016-12-06 14:01

manager   ~0007419

Is 'localObjectNames_' needed by 'writeObjects'? It appears that 'writeObjects' overrides the code relating to 'localObjectNames_' indicating that it is not general and only needed by derived classes providing a list of 'local' names suggesting the need for a an intermediate class handling the filtering of the 'local' names.

henry

2016-12-06 14:03

manager   ~0007420

How about 'writeLocalObjects' derived from 'writeObjectsBase'?

wyldckat

2016-12-06 14:30

updater   ~0007421

> If the names of the fields to write are in 'localObjectNames_' why is 'writeObjectNames_' needed?

'writeObjectNames_' acts as the mask for the fields that should be written. This meant to be used by 'yPlus' and other similar function objects.


> Is 'localObjectNames_' needed by 'writeObjects'?

No, it's not need for 'writeObjects'. I was aiming to have only a single new class, which was why this class has the two alternative uses:
 - for 'yPlus' et al, has the basis for managing local objects;
 - for 'writeObjects', has the common structure.


> How about 'writeLocalObjects' derived from 'writeObjectsBase'?

Yes, that does make more sense! That way it will be more specialized and not have the excess name management methods in 'writeObjects' stack.

I can take care of that this Thursday (holiday here in Portugal) and I'll also extend the use of 'writeLocalObjects' to the function objects that can gain from this feature.

henry

2016-12-06 14:41

manager   ~0007422

Sounds good, I think this is comming to gether well.

wyldckat

2016-12-09 01:15

updater   ~0007438

Third proposition is done and tested! The attached file 'proposition_dev.v3.tar.gz' provides the following files for OpenFOAM-dev:

 - src/OpenFOAM/Make/files
   - Adds 'writeLocalObjects' and 'writeObjectsBase'.

 - src/OpenFOAM/db/functionObjects/writeObjectsBase/writeObjectsBase.C
 - src/OpenFOAM/db/functionObjects/writeObjectsBase/writeObjectsBase.H

   - This class now has most of the base needed for 'writeObjects', while maintaining relevance for other function objects that may need this kind of base.
   - For example, the method 'objectNames()' provides the list of names for objects that are caught by the list set in 'writeObjectNames_', which use to be something that only 'writeObjects' did on its own.
   - The methods were prepared so that when they are overridden, then can still be re-used by the classes that inherit this class.
   - 'objects' keyword in the dictionary is obligatory.

 - src/OpenFOAM/db/functionObjects/writeLocalObjects/writeLocalObjects.C
 - src/OpenFOAM/db/functionObjects/writeLocalObjects/writeLocalObjects.H

   - New class that inherits from 'writeObjects', but overrides the method 'objectNames()' so that it only provides the result based on the existing list of local objects from the inheriting class.
   - 'read()' is also overridden, so that 'objects' keyword becomes is optional.

 - src/functionObjects/utilities/writeObjects/writeObjects.C
 - src/functionObjects/utilities/writeObjects/writeObjects.H

   - Now inherits a good chunk of the necessary code from 'writeObjectsBase'.
   - It pretty much only needs to override 'writeObject()' from 'writeObjectsBase'.
   - It calls a lot of the base methods from 'writeObjectsBase' for consistency.

 - src/functionObjects/field/yPlus/yPlus.C
 - src/functionObjects/field/yPlus/yPlus.H

   - Relies on 'writeLocalObjects' to handle the writing of the 'yPlus' field.
   - Description has been updated to reflect the new feature.

 - src/functionObjects/field/wallHeatFlux/wallHeatFlux.C
 - src/functionObjects/field/wallHeatFlux/wallHeatFlux.H

   - Relies on 'writeLocalObjects' to handle the writing of the 'wallHeatFlux' field.
   - Description has been updated to reflect the new feature.

 - src/functionObjects/field/wallShearStress/wallShearStress.C
 - src/functionObjects/field/wallShearStress/wallShearStress.H

   - Relies on 'writeLocalObjects' to handle the writing of the 'wallShearStress' field.
   - Description has been updated to reflect the new feature.


Warning: the message of the 3 function objects in field now refer to 'object' instead of 'field', e.g.:

    yPlus yplusFO write:
        writing object yPlus
        patch upperWall y+ : min = 3.32374, max = 8.1024, average = 6.5724
        patch lowerWall y+ : min = 1.73629, max = 32.8176, average = 20.2739

This is because it makes it consistent with 'writeObjects', e.g.:

    yPlus yplusFO write:
        patch upperWall y+ : min = 3.32374, max = 8.1024, average = 6.5724
        patch lowerWall y+ : min = 1.73629, max = 32.8176, average = 20.2739

    writeObjects writeYPlusFO write:
        writing object yPlus


I'm also attaching the following packages, which I used to test these 4 function objects:

 - 'pitzDaily.test_YPlus_WSS.tar.gz' - for testing 'yPlus' and 'wallShearStress', adapted from the tutorial case 'incompressible/simpleFoam/pitzDaily'.
 - 'hotRoom.test_wallHeatFlux.tar.gz' - for testing 'wallHeatFlux', adapted from the tutorial case 'heatTransfer/buoyantPimpleFoam/hotRoom'.

proposition_dev.v3.tar.gz (12,727 bytes)

wyldckat

2016-12-09 01:15

updater  

hotRoom.test_wallHeatFlux.tar.gz (2,990 bytes)

wyldckat

2016-12-09 01:15

updater  

pitzDaily.test_YPlus_WSS.tar.gz (5,208 bytes)

wyldckat

2016-12-09 01:33

updater   ~0007439

Side notes:

  1. From what I could see, there are a few other function objects that could potentially use 'writeLocalObjects', but:

    - Some of them would require changing their structure from doing everything in 'write()' to splitting it between 'execute()' and 'write()'. For example 'dsmcFields'.

    - Others can write several fields - e.g. 'nearWallFields' and 'surfaceInterpolate' - but it requires a bit more restructuring of their code in order to write their fields on-demand, given that they rely on their own template based handling.

  2. It's still missing an announcement of sorts regarding which fields can be selectively written locally or not. I didn't add this in, because I haven't found any function object that does this during construction.
    - Note: This would be useful for the previous note.


Either one of these should be done associated to another report, specially because it's still unclear (at least to me) how useful it would be for these function objects to have this feature.

henry

2016-12-09 22:01

manager   ~0007451

Thanks Bruno, your final proposition resolves the issue elegantly.

Resolved in OpenFOAM-dev by commit 249c09933e313879681d63e03b1771c2bef47ca6

joegi

2016-12-09 22:51

reporter   ~0007452

My official production version if 4.x, does this solution works in this version?

henry

2016-12-09 23:15

manager   ~0007453

Probably but it a fairly substantial development which changes the interface slightly and requires further testing and integration into some other functionObjects so I am not to planning to include it in OpenFOAM-4.x, at least not at the moment.

Issue History

Date Modified Username Field Change
2016-11-25 16:46 joegi New Issue
2016-11-25 17:08 wyldckat Relationship added related to 0002320
2016-11-26 17:19 wyldckat Note Added: 0007332
2016-11-26 17:35 henry Note Added: 0007333
2016-11-26 17:49 joegi Note Added: 0007334
2016-11-26 18:05 wyldckat Relationship added related to 0002355
2016-11-26 18:11 wyldckat Note Added: 0007335
2016-11-26 18:27 wyldckat Note Added: 0007337
2016-11-26 19:11 wyldckat Note Added: 0007338
2016-11-26 19:50 wyldckat Note Added: 0007339
2016-11-26 20:07 henry Note Added: 0007341
2016-11-26 20:12 henry Note Added: 0007342
2016-11-26 20:32 wyldckat Note Added: 0007343
2016-11-26 20:50 henry Note Added: 0007345
2016-11-27 23:21 wyldckat File Added: yPlus.4x.writeFieldHack.tar.gz
2016-11-27 23:21 wyldckat Note Added: 0007347
2016-12-04 11:14 wyldckat File Added: proposition_dev.v1.tar.gz
2016-12-04 11:14 wyldckat Note Added: 0007391
2016-12-04 11:23 henry Note Added: 0007392
2016-12-04 13:37 wyldckat Note Added: 0007393
2016-12-04 15:13 henry Note Added: 0007394
2016-12-04 15:26 henry Note Added: 0007395
2016-12-04 16:06 wyldckat Note Added: 0007396
2016-12-04 16:21 henry Note Added: 0007397
2016-12-04 16:45 wyldckat Note Added: 0007398
2016-12-04 17:07 henry Note Added: 0007399
2016-12-04 17:27 wyldckat Note Added: 0007400
2016-12-04 22:57 wyldckat Note Added: 0007401
2016-12-04 22:58 wyldckat File Added: proposition_dev.v2.tar.gz
2016-12-06 12:42 henry Note Added: 0007415
2016-12-06 13:03 wyldckat Note Added: 0007416
2016-12-06 13:30 henry Note Added: 0007417
2016-12-06 14:01 henry Note Added: 0007419
2016-12-06 14:03 henry Note Added: 0007420
2016-12-06 14:30 wyldckat Note Added: 0007421
2016-12-06 14:41 henry Note Added: 0007422
2016-12-09 01:15 wyldckat File Added: proposition_dev.v3.tar.gz
2016-12-09 01:15 wyldckat Note Added: 0007438
2016-12-09 01:15 wyldckat File Added: hotRoom.test_wallHeatFlux.tar.gz
2016-12-09 01:15 wyldckat File Added: pitzDaily.test_YPlus_WSS.tar.gz
2016-12-09 01:33 wyldckat Note Added: 0007439
2016-12-09 22:01 henry Assigned To => henry
2016-12-09 22:01 henry Status new => resolved
2016-12-09 22:01 henry Resolution open => fixed
2016-12-09 22:01 henry Fixed in Version => dev
2016-12-09 22:01 henry Note Added: 0007451
2016-12-09 22:51 joegi Status resolved => feedback
2016-12-09 22:51 joegi Resolution fixed => reopened
2016-12-09 22:51 joegi Note Added: 0007452
2016-12-09 23:15 henry Note Added: 0007453
2016-12-09 23:15 henry Status feedback => closed
2016-12-09 23:15 henry Resolution reopened => fixed