View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0002090OpenFOAMBugpublic2016-05-15 01:132016-08-17 14:43
Reporterwyldckat Assigned Tohenry  
PrioritylowSeverityfeatureReproducibilityN/A
Status resolvedResolutionfixed 
Product Versiondev 
Fixed in Versiondev 
Summary0002090: Recent commit makes it not easy to use "secondary time" write options
DescriptionI noticed that the commit 758dfc2c1 has the following note:

  Also removed the redundant secondary-writing functionality from Time which has been superseded by the 'writeRegisteredObject' functionObject.

Assuming that I've interpreted the source code correctly on the latest commit 26658647fa4be, the 'writeRegisteredObject' functionObject does not provide an easy way to reproduce this functionality.

If I understood correctly the reason for the secondary write option (which has been now removed), it was so that it could support, for example, the ability to take a time snapshot at cpuTime, if the user needs to account for a time limit on an HPC solution that restricts the total execution time for a parallel run.

I've taken a look into the code for 'writeRegisteredObject' and it seems to just be a matter of:

  1- Adding a new option that allows choosing the type of registered objects based on their own write option, namely "AUTO_WRITE", "NO_WRITE" or "any".

    - Note: This would also require the other option "exclusiveWriting" to be false when the new option isn't "any".

  2- Giving the example that relies on the new option and using (".*") as the way for selecting field names.

  3- The ability to purge previous time steps that this function object writes might be useful as well, but it doesn't feel as important as the need to be able to easily reproduce the deprecated functionality.
Additional InformationIf no-one beats be to it, I'll have a proposition for this feature uploaded as soon as I can write it and test it, which might not be this weekend :(
TagsNo tags attached.

Activities

henry

2016-05-15 09:04

manager   ~0006273

Please go ahead and implement the additional functionality 'writeRegisteredObject' you propose.

Note that I am currently completely restructuring functionObjects to make them simpler to implement, easier to maintain, more consistent and more comprehensible. The basic work on this should be completed later today or tomorrow after which I will work on cleaning up individual functionObjects and complete some new ones I am working on and it would be good if you take on the generalization of 'writeRegisteredObject'.

henry

2016-05-15 16:55

manager   ~0006274

See commit 91aba2db2ecba575d4241198a75ff3ee983ee421

    functionObjects: rewritten to all be derived from 'functionObject'
    
      - Avoids the need for the 'OutputFilterFunctionObject' wrapper
      - Time-control for execution and writing is now provided by the
        'timeControlFunctionObject' which instantiates the processing
        'functionObject' and controls its operation.
      - Alternative time-control functionObjects can now be written and
        selected at run-time without the need to compile wrapped version of
        EVERY existing functionObject which would have been required in the
        old structure.
      - The separation of 'execute' and 'write' functions is now formalized in the
        'functionObject' base-class and all derived classes implement the
        two functions.
      - Unnecessary implementations of functions with appropriate defaults
        in the 'functionObject' base-class have been removed reducing
        clutter and simplifying implementation of new functionObjects.
      - The 'coded' 'functionObject' has also been updated, simplified and tested.
      - Further simplification is now possible by creating some general
        intermediate classes derived from 'functionObject'.

You can now work on the 'writeRegisteredObject' whenever you have time.

wyldckat

2016-07-03 23:35

updater   ~0006488

In the report #2138 I've already provided a few typo fixes, but for this feature contribution, there is something I need to ask first, regarding the following option: the "exclusiveWriting" option seems to be redundant, given that we should instead cross-check with the primary "runTime" variable from the solver (through the registry connection), to know if we need to write an auto-write object for the current time step or not.

So the question is: Can we simply remove the "exclusiveWriting" option in 4.x as well, once we rely on the primary "runTime" for reference?
I ask this to know if I should provide two sets of patches (4.x and dev) or just one for the development repository.


Technically, there should be a way to account for a field that has already been written by another function object or the main "runTime" loop for the current time step, but for now this seems a bit like overkill, since the end-users should be able to sort this out on their own, i.e. to not have function objects competing to write the same file.

henry

2016-07-04 08:12

manager   ~0006489

Yes the "exclusiveWriting" option is redundant and can be removed from 4.x and dev.

wyldckat

2016-08-16 02:32

updater  

writeObjects_v1.tar.gz (2,813 bytes)

wyldckat

2016-08-16 02:51

updater   ~0006671

After 3 months, I've finally managed to take a proper look into this and to provide the attached proposition package "writeObjects_v1.tar.gz" for OpenFOAM 4.x and dev, which has the following updated files:

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

It includes the following changes:

  1. Removed the "exclusiveWriting" option, since it's a redundant option; it has been replaced by checking automatically whether an object is being written automatically and if "obr_.time().writeTime()" is true.

  2. Added a new option named "writeOption", which is documented as follows:

    It also has the ability to write the selected objects that were defined
    with the respective write mode for the requested \c writeOption, namely:
      - \c autoWrite - objects set to write at output time
      - \c noWrite - objects set to not write by default
      - \c anyWrite - any option of the previous two

  3. "anyWrite" is the default option when "writeOption" is omitted.


In order to do a catch-all test, I've used the tutorial case "heatTransfer/chtMultiRegionSimpleFoam/heatExchanger" and appended the following block to the file "system/controlDict":

    functions
    {
        writeObjects1
        {
            type writeObjects;
            libs ("libutilityFunctionObjects.so");
            region air;
            writeControl timeStep;
            writeInterval 5;
            objects (".*");
            writeOption autoWrite;
        }

        writeObjects2
        {
            type writeObjects;
            libs ("libutilityFunctionObjects.so");
            region air;
            writeControl timeStep;
            writeInterval 3;
            objects (".*");
            writeOption noWrite;
        }
    }


But this triggered a weird bug, which I'm still trying to figure out. The issue has to do with the "noWrite" option (even when "writeObjects1" is commented out), which is not being fully respected... resulting in also writing files that are set to auto-write, such as "*/air/U", but these are not logged by the updated "writeObjects" code.

I still need to investigate further on this latest issue... I'll report back once I've done a more detailed analysis of what's going on... because right now, the only possibility that comes to mind is that this call "obr_.time().writeTimeDict()" is writing also auto-written files, instead of just the "uniform" folder...

wyldckat

2016-08-16 13:48

updater   ~0006678

I've managed to figure out the problem when we use the new entry "writeOption noWrite". There are at least 3 special registered objects that will write all auto-written files if we ask any of those 3 to be written:

  - data
  - fvSchemes
  - fvSolution


So my question here is: What is the preferable approach to sorting out this issue?
I ask this because I haven't found yet anything that helps identify these special objects at the "registered object" level.

I have in mind a few possibilities, but so far I'm not 100% certain on any one of them to be implemented:

  1. Hard-code in this function object that these 3 objects should never be written.

    - Pro: Quickest to implement.
    - Con: This isn't a good policy and would lead to confusing maintenance issues.


  2. Have an optional counter-list for objects that should not be written.

    - Pros:

      - It's useful, even for when people want to "catch all except one or two others", without having to go into very complex regular expressions...
      - We can set as default that the 3 special objects are part of this counter-list and document accordingly.

    - Con: We can still have future maintenance issues, but at least people can workaround it until it's fixed.


  3. To have an additional entry on "IOobject" that allows us to tag it as "object group" or "special group" or something similar. (This is assuming I didn't miss this feature already existing in OpenFOAM...)

    - Pro: May come in handy for other features.

    - Cons:

      - There might be implementation repercussions that I'm not anticipating.
      - Still has a high potential for maintenance issues.

wyldckat

2016-08-16 21:41

updater  

writeObjects_v2.tar.gz (3,254 bytes)

wyldckat

2016-08-16 22:00

updater   ~0006681

I went with the 2nd option from the previous comment, given that it's the most flexible one.

I've attached the revised+updated code, inside the package "writeObjects_v2.tar.gz" for OpenFOAM 4.x and dev, which has the following updated files:

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


In review, it includes the following changes:

  1. Removed the "exclusiveWriting" option, since it's a redundant option; it has been replaced with checking automatically whether an object is being written automatically and if "obr_.time().writeTime()" is true.

  2. Added a new option named "writeOption", which is documented as follows:

      It also has the ability to write the selected objects that were defined
      with the respective write mode for the requested \c writeOption, namely:
      \vartable
          autoWrite | objects set to write at output time
          noWrite | objects set to not write by default
          anyWrite | any option of the previous two
      \endvartable

    "anyWrite" is the default option when "writeOption" is omitted, which is documented in the Usage section.
    The implementation is relying on a 'NamedEnum', to be consistent with OpenFOAM's way of handling these enumerations. Couldn't use the "IOobject::writeOption" enumeration directly, given "anyWrite" was needed as a new option.

  3. Added the optional entry "skipObjects", which serves as a counter-list to "objects". If not defined, it's set by default to "(data fvSchemes fvSolution)".

  4. Revised the Description section accordingly to the new features.

  5. 'writeObjects::write()' now only writes the objects that abide to the desired options.


This way the "secondary-writing functionality" is mostly brought back, although purging past time snapshots is not done with this function object. To do so, we can for example use the following function object entry in "controlDict":

    auxiliaryWrite
    {
        type writeObjects;
        libs ("libutilityFunctionObjects.so");
        writeControl cpuTime;
        writeInterval 3600.0;
        objects (".*");
        writeOption autoWrite;
    }


The other limitation is that this will require multiple entries, one per region, in order to be able to have the "secondary-writing functionality" on all regions, for example:

    auxiliaryWrite_air
    {
        $auxiliaryWrite;
        region air;
    }

    auxiliaryWrite_porous
    {
        $auxiliaryWrite;
        region porous;
    }

henry

2016-08-16 22:40

manager   ~0006682

Are you sure you are solving a 'real' problem? Why would you want to write all objects which are set NO_WRITE? Apart from the classes fvMesh is derived from there are lots of other issues with writing all NO_WRITE, in particular most of the case dictionaries are re-written without comments etc. for no useful reason.

Why not simply specify the particular field you want written which are set NO_WRITE?

henry

2016-08-16 23:47

manager   ~0006683

I merged in and tested your initial proposal:

OpenFOAM-dev commit 7238bd101628b4838a8c179154ca149534eb30a5

and do not see any problem with it or a need for the additional complexity of the "skipObjects" option you subsequently added. The selection of the object to write is already with wildcards which provides sufficient flexibility in other contexts in OpenFOAM I don't think it needs to different for this functionObject.

wyldckat

2016-08-17 00:19

updater   ~0006684

Last edited: 2016-08-17 00:21

 I did have to restore several files in the constant folder on the tutorial cases I was testing with, in order to do several tests with the "NO_WRITE" option, which was why I ended up with the v2 solution.

Essentially I tried to effectively deal with all branching scenarios of using ".*" along with each option in "writeOption". And since the "noWrite" option revealed some issues where the users are likely to shoot themselves in the foot, I wanted to ensure that at least the means are provided to easily allow the user to fix the error in another attempt.

Having the ability to write most or all objects that are set to "NO_WRITE" is mostly for debugging purposes. For example, in the tutorial "incompressible/simpleFoam/pitzDaily", the streamlines function object is used, which creates additional registered objects; these objects are first created at write time, which means that knowing about them ahead of time is not achievable without doing a previous run. And having access to their values can come in handy for debugging the calculations being done.

Therefore, in this kind of debugging situation, using ".*" with "noWrite" makes it straight forward to catch all objects, namely those that we want to catch and don't know their names ahead of time. And since having exclusions directly in the regular expressions in the "objects" list is not straight forward, it seemed to make more sense to add the counter-list with a few default values already in.

-------

Anyway, I do understand your point of view (less to maintain and it's general enough already) and if you prefer the v1 approach, I need to re-check some of the fixes I did in v2 and provide a few fixes for v1 (mostly text and coding style).

henry

2016-08-17 07:36

manager   ~0006685

Rather than add support for a "counter-list" everywhere where regular expressions are supported wouldn't it be easier and more general to extend the regular expressions to handles exceptions in the list in a convenient manner?

For the moment I am happy with v1 and have integrated it into OpenFOAM-dev with some minor changes. Please provide the changes you would like to make against the version in OpenFOAM-dev. Thanks.

wyldckat

2016-08-17 12:13

updater  

writeObjects_v3.tar.gz (2,889 bytes)

wyldckat

2016-08-17 12:29

updater   ~0006687

I've attached the package "writeObjects_v3.tar.gz", which is built on top of v1 and has the following updates:

  - Renamed the variable "writeOptionNames" to "writeOptionNames_", based on what I saw on another class that uses NamedEnum.

  - Simplified a bit the "switch" block in "write()", along with following the bracket based blocking for the "case" entries and used FatalErrorInFunction for the "default" entry.

  - Minor revision of the Description in the header file (use \vartable instead of list).

  - Repaired description for "writeOption_".

This patch should wrap up this report.

-----

> Rather than add support for a "counter-list" everywhere where regular expressions are supported wouldn't it be easier and more general to extend the regular expressions to handles exceptions in the list in a convenient manner?

Mmm... I didn't think of that, because it seemed preferable to stick to the regular expressions standard. But it does make more a lot more sense to extend the current regex support to something that easily allows exceptions in patterns. Although the only solution I've found is using the same convention that 'sed' uses, e.g.: "/fv.*/!;/.*/"

henry

2016-08-17 14:43

manager   ~0006691

Resolved by commit 16804b9d6fd45311afd441f893d628fcb4e2d2b1

Issue History

Date Modified Username Field Change
2016-05-15 01:13 wyldckat New Issue
2016-05-15 09:04 henry Note Added: 0006273
2016-05-15 16:55 henry Note Added: 0006274
2016-07-03 23:35 wyldckat Note Added: 0006488
2016-07-04 08:12 henry Note Added: 0006489
2016-08-16 02:32 wyldckat File Added: writeObjects_v1.tar.gz
2016-08-16 02:51 wyldckat Note Added: 0006671
2016-08-16 13:48 wyldckat Note Added: 0006678
2016-08-16 21:41 wyldckat File Added: writeObjects_v2.tar.gz
2016-08-16 22:00 wyldckat Note Added: 0006681
2016-08-16 22:00 wyldckat Assigned To => henry
2016-08-16 22:00 wyldckat Status new => assigned
2016-08-16 22:40 henry Note Added: 0006682
2016-08-16 23:47 henry Note Added: 0006683
2016-08-17 00:19 wyldckat Note Added: 0006684
2016-08-17 00:21 wyldckat Note Edited: 0006684
2016-08-17 07:36 henry Note Added: 0006685
2016-08-17 12:13 wyldckat File Added: writeObjects_v3.tar.gz
2016-08-17 12:29 wyldckat Note Added: 0006687
2016-08-17 14:43 henry Note Added: 0006691
2016-08-17 14:43 henry Status assigned => resolved
2016-08-17 14:43 henry Fixed in Version => dev
2016-08-17 14:43 henry Resolution open => fixed