View Issue Details

IDProjectCategoryView StatusLast Update
0003781OpenFOAMBugpublic2022-01-14 18:09
Reporterivor.clifford Assigned Tohenry  
PrioritylowSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformGNU/LinuxOSUbuntuOS Version15.04
Product Version9 
Fixed in Version9 
Summary0003781: Mismatch between user and physical time when adapting the time precision
DescriptionI am developing a new solver in which I derive from Foam::Time and override the virtual functions Foam::TimeState::userTimeToTime, etc. While doing this, I noticed that OpenFOAM unexpectedly increases the write precision, which seems to be due to mismatches between user- and physical time in OpenFOAM's time classes.

After some debugging, I think I found the issue. In Time.C:1287

        // Tolerance used when testing time equivalence
        const scalar timeTol =
            max(min(pow(10.0, -precision_), 0.1*deltaT_), SMALL);

Here the physical time is used to define the tolerance for increasing the time precision, but a few lines down (1302) this is compared against the user time. This mismatch causes the precision to unexpectedly increase.
A second issue I noted is that, on line 1292, userDeltaT is determined using the function timeToUserTime. This is only correct if the user time is a linear function of the physical time. It would be better to calculate this as:

    const scalar userDeltaT = timeToUserTime(value()) - timeToUserTime(value() - deltaT_);
Steps To ReproduceThis bug is not related to any specific usage of the code.
TagsNo tags attached.

Activities

henry

2022-01-13 13:55

manager   ~0012381

I have completely rewritten the handling of user-time in OpenFOAM-dev, I think it would make sense for you to upgrade to OpenFOAM-dev before writing code relating to user-time to avoid having to rewrite it:

commit 3ef3e96c3f217baaeaaebb7e4f94593354dff069
Author: Henry Weller <http://cfd.direct>
Date: Tue Oct 19 09:09:01 2021 +0100

    Time: Added run-time selectable userTime option
    
    replacing the virtual functions overridden in engineTime.
    
    Now the userTime conversion function in Time is specified in system/controlDict
    such that the solver as well as all pre- and post-processing tools also operate
    correctly with the chosen user-time.
    
    For example the user-time and rpm in the tutorials/combustion/XiEngineFoam/kivaTest case are
    now specified in system/controlDict:
    
    userTime
    {
        type engine;
        rpm 1500;
    }
    
    The default specification is real-time:
    
    userTime
    {
        type real;
    }
    
    but this entry can be omitted as the real-time class is instantiated
    automatically if the userTime entry is not present in system/controlDict.

henry

2022-01-13 14:00

manager   ~0012382

I can implement a correction to the code but I need a means to test it, can you provide a case which reproduces the problem?

henry

2022-01-13 14:04

manager   ~0012383

Last edited: 2022-01-13 14:05

> This is only correct if the user time is a linear function of the physical time.

Are you planning to use a user time which is not a linear function of the physical time? The reason I ask is that while the user-time handling in Time does not assume a linear relationship there are a couple of pieces of code relating to user time which currently do and it was not clear if this would ever be an issue.

ivor.clifford

2022-01-13 14:21

reporter   ~0012384

We will try to migrate to the dev version when convenient, thank you. I think this approach is an improvement. Looking at Time.C in OpenFOAM-dev, however, I think the issue still exists.

To provide a test case, I could play with the engine solvers in an attempt to trigger the problem. Would this be sufficient?

> Are you planning to use a user time which is not a linear function of the physical time?
This is just an idea we are considering. In some cases, we have experimental measurements at arbitrary time intervals that we refer to as burnup steps. For these problems, it would be useful to monitor the evolution in terms of these burnup steps rather than physical time.

henry

2022-01-13 15:42

manager   ~0012385

Last edited: 2022-01-13 15:59

> Looking at Time.C in OpenFOAM-dev, however, I think the issue still exists.

Correct, I wasn't suggesting the issue had been fixed in dev, only that the user-time mechanism is completely different and better.

> To provide a test case, I could play with the engine solvers in an attempt to trigger the problem. Would this be sufficient?

Yes

Presumably

            // User-time equivalent of deltaT
            const scalar userDeltaT =
                timeToUserTime(value()) - timeToUserTime(value() - deltaT_);

            // Tolerance used when testing time equivalence
            const scalar timeTol =
                max(min(pow(10.0, -precision_), 0.1*userDeltaT), small);

is sufficient to fix the problem?

My feeling is a non-linear user-time to time relationship is not sensible and I did consider in the re-write in OpenFOAM-dev to fundamentally limit the conversion to being linear but I couldn't be sure that a non-linear relationship wouldn't be needed for some purpose. If this is needed then other parts of the code will need to be updated to support it.

ivor.clifford

2022-01-13 15:54

reporter   ~0012386

Dear Henry,
I have prepared a case to trigger the problem. This is simply a modified version of kivaTest where the engine speed is reduced to 1e-3 rpm. The large ratio of real deltaT to angular deltaTheta triggers the problem. If we run this case using XiEngineFoam, the time precision increases by 3 at each writeTime leading to a final time precision at the end of the simulation of 18, i.e. machine precision.

> Presumably ... is sufficient to fix the problem?
Your proposed fix would likely work, yes.

> My feeling is a non-linear user-time to time relationship is not sensible
There's no pressure from my side to support non-linear relationships. It's almost better to know upfront that we should not pursue this.

Kind Regards
Ivor
kivaTest_bug.tar.gz (1,073,276 bytes)

henry

2022-01-13 16:02

manager   ~0012387

Actually

            // User-time equivalent of deltaT
            const scalar userDeltaT =
                timeToUserTime(value()) - timeToUserTime(value() - deltaT_);

is even needed to handle linear time conversion if there is an offset in the user-time to time conversion which is supported.

henry

2022-01-14 16:30

manager   ~0012410

I have isolated the issue and am testing a fix in OpenFOAM-9 and if all is well will make the corresponding change to OpenFOAM-dev.

henry

2022-01-14 18:09

manager   ~0012411

Resolved in OpenFOAM-9 by commit 6afbfbe27a63ce9c89d64788837d58338b932681 and eca6d201d3db996a06a8c3c30ea7e6aed36c2eec
Resolved in OpenFOAM-dev by commit fca30da1fd8d8035e866c9d3023f7f5a2afb9647 and 76d8280c9642dc0b73c3abe4083070eec7010dbe

Issue History

Date Modified Username Field Change
2022-01-13 13:07 ivor.clifford New Issue
2022-01-13 13:55 henry Note Added: 0012381
2022-01-13 14:00 henry Note Added: 0012382
2022-01-13 14:04 henry Note Added: 0012383
2022-01-13 14:05 henry Note Edited: 0012383
2022-01-13 14:21 ivor.clifford Note Added: 0012384
2022-01-13 15:42 henry Note Added: 0012385
2022-01-13 15:54 ivor.clifford File Added: kivaTest_bug.tar.gz
2022-01-13 15:54 ivor.clifford Note Added: 0012386
2022-01-13 15:59 henry Note Edited: 0012385
2022-01-13 16:02 henry Note Added: 0012387
2022-01-14 16:30 henry Note Added: 0012410
2022-01-14 18:09 henry Assigned To => henry
2022-01-14 18:09 henry Status new => resolved
2022-01-14 18:09 henry Resolution open => fixed
2022-01-14 18:09 henry Fixed in Version => 9
2022-01-14 18:09 henry Note Added: 0012411