View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0000815 | OpenFOAM | Bug | public | 2013-04-15 08:32 | 2015-03-22 09:53 |
| Reporter | projectionist | Assigned To | henry | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Platform | Ubuntu | OS | Linux | OS Version | 12.04 |
| Summary | 0000815: Time step directory naming error | ||||
| Description | If I continue a simulation, e.g. it first it ran to 20s then I continue to 60s, the time step directory names get a bit weird. OpenFOAM issues some warning messages about increasing the timePrecision. You find my controlDict attached. I also included some lines of the solver output. The time step directory names look like this: 118.5000000003 119.0000000003 119.5000000003 120.0000000003 60 60.5 61 61.5 In this case the simulation was run till t=60s and then continued to t=120s. | ||||
| Steps To Reproduce | restart a simulation. Build : 2.1.x-82fc6e06f8b3 Build : 2.1.x-0038d7805653 In both cases I made simulation with fixed time step. | ||||
| Additional Information | Courant Number mean: 0.000730585 max: 0.00230961 Max Ur Courant Number = 0.00375471 Time = 20.0001 --> FOAM Warning : From function Time::operator++() in file db/Time/Time.C at line 1024 Increased the timePrecision from 6 to 7 to distinguish between timeNames at time 20 MULES: Solving for alpha1 /* solver output */ ExecutionTime = 3.23 s ClockTime = 4 s Courant Number mean: 0.000730578 max: 0.00230962 Max Ur Courant Number = 0.00375456 --> FOAM Warning : From function Time::operator++() in file db/Time/Time.C at line 1024 Increased the timePrecision from 7 to 17 to distinguish between timeNames at time 20.0001 Time = 20.000100000000003 MULES: Solving for alpha1 /* solver output */ ExecutionTime = 5 s ClockTime = 6 s Courant Number mean: 0.000730579 max: 0.00230963 Max Ur Courant Number = 0.00375451 Time = 20.000150000000005 MULES: Solving for alpha1 | ||||
| Tags | No tags attached. | ||||
|
|
controlDict (1,478 bytes)
/*--------------------------------*- C++ -*----------------------------------*\
| ========= | |
| \\ / F ield | OpenFOAM: The Open Source CFD Toolbox |
| \\ / O peration | Version: 2.1.x |
| \\ / A nd | Web: www.OpenFOAM.org |
| \\/ M anipulation | |
\*---------------------------------------------------------------------------*/
FoamFile
{
version 2.0;
format ascii;
class dictionary;
location "system";
object controlDict;
}
// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
application twoPhaseEulerFoam;
//startFrom startTime;
startFrom latestTime
startTime 0;
stopAt endTime;
endTime 20;
deltaT 0.00005;
writeControl runTime;
//writeControl adjustableRunTime;
writeInterval 0.5;
purgeWrite 0;
writeFormat ascii;
writePrecision 6;
writeCompression uncompressed;
timeFormat general;
timePrecision 6;
runTimeModifiable off;
adjustTimeStep no;
maxCo 0.5;
maxDeltaT 1;
functions
{
probes1
{
type probes;
functionObjectLibs ("libsampling.so");
dictionary probesDict;
}
}
// ************************************************************************* //
|
|
|
See also: http://openfoam.org/mantisbt/view.php?id=514 |
|
|
You start off from 20 with a deltaT of 0.00005 so you'll need at least 2 (leading digits) + 5 (trailing digits) time precision. I do not understand the second, 7 to 17 digits precision extension - do you have a 22x testcase? |
|
|
This issue seems related to this one: http://www.openfoam.org/mantisbt/view.php?id=1557 |
|
|
|
|
|
OK, step 1: reproduce problem - complete! Attached is the file "runTimeSteps.tar.gz", which has packaged the test utility "runTimeSteps" (ready for "applications/test") which compiles to the binary named "Test-runTimeSteps". Do feel free to add it to the test folder, since this is extremely basic stuff (and I snipped most of it anyway from "simpleFoam" and "pimpleFoam")! How to use it, after unpacking: wmake cd pitzDailyDummy blockMesh Then: Test-runTimeSteps -simple > log Test-runTimeSteps -simple > log2 Or: Test-runTimeSteps -pimple > log Test-runTimeSteps -pimple > log2 The reason for the two types was for the simple reasoning of "what if something is different between simple and pimple". This way it's ready for adding in a few more tests, if need be (e.g. deltaT affected by "CourantNo"). The reason why we should run twice is because of these settings: startFrom latestTime; startTime 0; stopAt endTime; endTime 100; deltaT 0.00005; writeControl runTime; writeInterval 0.54325; The first run will end at "99.958" and everything looks fine. The second run will reproduce the reported problem: Create mesh for time = 99.958 SIMPLE: no convergence criteria found. Calculations will run for 100 steps. Starting time loop Time = 99.9581 --> FOAM Warning : From function Time::operator++() in file db/Time/Time.C at line 1055 Increased the timePrecision from 6 to 17 to distinguish between timeNames at time 99.9581 Time = 99.958100000000002 Time = 99.958150000000003 Notice anything else strange? "99.95805" is missing in between load and 1st step! Which probably explains why the iterator went "uh oh, gotta fix this" - but did it way too late and messed it up ;) --------------- Next step: figuring out the fix for this. |
|
|
|
|
|
|
|
|
Steps 2 and 3: Diagnosis and first possible fix - completed! Relative to commit ade709a58 from 2.3.x. Diagnosis: Comparing the old name with the new name, for the time snapshots, results in not taking into account the limitations of the number rounding up/down algorithm. In the example from the previous comment, "99.95805" is rounded up to "99.9581", which results in colliding with the actual "99.9581" in the next iteration. Solution: Compare the actual values, namely the currently defined value - reinterpreted from string - versus the previous value and compare the difference to the desired "deltaT". First possible fix: Attached are two files: - "Time.C.v1" - the complete modified file "src/OpenFOAM/db/Time/Time.C", since patches aren't always easy to use. - "issue815.patch.v1" - the patch file that should result in the file "Time.C.v1", for easier on-the-fly inspection. This fix doesn't seem to the best possible solution, but right now I can't deduce a better one. In addition, this code needs aesthetic revising, since I wasn't certain how to follow the guidelines in the situations of long "if" and "while" statements. If it's possible that we could access directly to the rounded up/down value at the time of formatting to "word", it would save up a few cpu cycles... |
|
|
This is close to the way I have been thinking about this problem, the only difference is that I think we need a better way of defining what an appropriate smallest time-step for the case might be (sqrt(SMALL) in your code if I understood it correctly). While it might be OK to define a default I think it sholud be possible to over-ride it and this also would allow for the case to terminate if the time-step is reduced below this limit. |
|
|
OK, I'll leave that detail to you. I'm finishing up a second additional patch, which relates mainly to issue #1557 and a little bit to this one as well. |
|
|
Well... looks like the 2nd patch I tried to created that is meant for #1557 cannot work properly without adding an additional argument to the "Time" constructors for checking if the folder exists. It's provided as attachment "issue815.patch.v2". Perhaps it makes more sense to add a new method to "Time" and then add this check to "createTime.H" that is used by most solvers? |
|
|
|
|
|
I don't see the need to change the code for #1557, if the time directory is not present the fields cannot be read from it and the code stops. Perhaps the message could be improved a bit as in your change to Time but I don't think Time should attempt the check as the directory may not be needed. |
|
|
Building on your idea of checking of the existence of the time directory why don't we instead attempt to work out what the precision needs to be set to find it: // Check if time directory exists // If not increase time precision to see if it is formatted differently. if (!exists(timePath(), false)) { int oldPrecision = precision_; bool found = false; for (precision_=oldPrecision; precision_<20; precision_++) { // Update the time formatting setTime(startTime_, 0); // Check the existence of the time directory with the new format found = exists(timePath(), false); if (found) break; } if (found) { WarningIn("Time::setControls()") << "Increasing the timePrecision from " << oldPrecision << " to " << precision_ << " to support the formatting of the current time directory " << timeName() << nl << endl; } else { // Could not find time directory so assume it is not present precision_ = oldPrecision; // Revert the time formatting setTime(startTime_, 0); } } Realistically what should the maximum precision be for this? I think that 100 use elsewhere is unnecessarily large. Probably 10 to cover most applications but should be be say 20 as in the above? |
|
|
I believe that maximum precision should be tied to the machine epsilon for the current scalar precision: http://en.wikipedia.org/wiki/Machine_epsilon - SP: minimum required: 7. But perhaps 9-10? - DP: minimum required: 16. Again, perhaps 19-20? A bit of extra precision would only be useful for corner cases like, e.g. for SP: 1.23456789999 I forgot to mention yesterday that the main problem with the 2nd patch is that several warnings are issued in cases where the folders are still being constructed, e.g. when using decomposePar. Which is why I suggested that the check should perhaps be done separately, namely only when we really need to folder to already exist. OK, now I see it. Yes, your code should work well and it will only warn us if the precision needed to be upped. |
|
|
I am working on your first patch at the moment. Having run your test case and put in some diagnostics I see exactly what is going on and how your patch resolves it -- thanks for effort. I hope to put something together which is a bit more elegant and obvious but it is not so easy converting word representations back to scalars as you already found. Your suggestion on SP vs DP makes sense, basically (3 -log10(SMALL)) or similar |
|
|
OK, found a corner case: The precision has to be searched for in the inverse direction, because of situations like these: - 99.958 - 99.95800001 It will stop searching when it finds the first folder. |
|
|
Good point, I will finish sorting out a static member specifying the maximum precision and then reverse the loop. |
|
|
Tested it just now... doesn't work as intended :( Inverse order leads to too much precision, which can lead to unwanted inaccuracies... Not found: "/home/ofuser/OpenFOAM/OpenFOAM-2.3.x/applications/test/runTimeSteps/pitzDailyDummy/1e+02" Patch checked: "/home/ofuser/OpenFOAM/OpenFOAM-2.3.x/applications/test/runTimeSteps/pitzDailyDummy/99.958", 1 Patch checked: "/home/ofuser/OpenFOAM/OpenFOAM-2.3.x/applications/test/runTimeSteps/pitzDailyDummy/99.9579999999999984", 0 --> FOAM Warning : From function Time::setControls() in file db/Time/Time.C at line 225 Increasing the timePrecision from 2 to 18 to support the formatting of the current time directory 99.9579999999999984 Create mesh for time = 99.9579999999999984 Unless it searches for all possible precisions... |
|
|
OK, I will take it out. |
|
|
Tested just now... here's what seemed to worked well: // Check if time directory exists // If not increase time precision to see if it is formatted differently. if (!exists(timePath(), false)) { int oldPrecision = precision_; bool found = false; int precisionNeeded = -1; for ( precision_ = 3-log10(SMALL); precision_ > oldPrecision; precision_-- ) { // Update the time formatting setTime(startTime_, 0); // Check the existence of the time directory with the new format found = exists(timePath(), false); if (found) { precisionNeeded = precision_; } } if (precisionNeeded>0) { precision_ = precisionNeeded; WarningIn("Time::setControls()") << "Increasing the timePrecision from " << oldPrecision << " to " << precision_ << " to support the formatting of the current time directory " << timeName() << nl << endl; } else { // Could not find time directory so assume it is not present precision_ = oldPrecision; // Revert the time formatting setTime(startTime_, 0); } } Searching in reverse has the advantage of calling "3-log10(SMALL)" only once. |
|
|
I have just created a maxPrecision_ static member so evaluation is not an issue. Feel free to search in either direction. |
|
|
Searching in direct order based in the code from my previous comment: precision_ = oldPrecision+1; precision_ <= 3-log10(SMALL); precision_++ fails to properly identify the minimal precision needed. Looks like reverse order is better for a simpler and more efficient code? Otherwise more "if"s have to be used, as far as I can figure out. |
|
|
Yes, I think searching in reverse-order is more logical. I am building the changes with maxPrecision_ at the moment; takes a while.... |
|
|
OK, with any luck this is the final code that works as intended... it was missing a setTime in the last if block: // Check if time directory exists // If not increase time precision to see if it is formatted differently. if (!exists(timePath(), false)) { int oldPrecision = precision_; bool found = false; int precisionNeeded = -1; for ( precision_ = 3-log10(SMALL); precision_ > oldPrecision; precision_-- ) { // Update the time formatting setTime(startTime_, 0); // Check the existence of the time directory with the new format found = exists(timePath(), false); if (found) { precisionNeeded = precision_; } } if (precisionNeeded>0) { precision_ = precisionNeeded; // Update the time formatting setTime(startTime_, 0); WarningIn("Time::setControls()") << "Increasing the timePrecision from " << oldPrecision << " to " << precision_ << " to support the formatting of the current time directory " << timeName() << nl << endl; } else { // Could not find time directory so assume it is not present precision_ = oldPrecision; // Revert the time formatting setTime(startTime_, 0); } } |
|
|
> it was missing a setTime in the last if block: I noticed that and put it in. |
|
|
OK, on this topic, I think I can still do today some additional tests for the two fixes that you already have got scheduled to be committed, if you want to push them first to "OpenFOAM-dev". ----- The 3rd and last topic I had in mind related to this was to make it "crystal clear" to the user from which time step the mesh is coming from... but it seems to require some considerable tweaking to the constructors of class "polyMesh". The main tweak would be to enforce that the core files for the mesh ("faces", "points", etc...) should all come from the same time folder... which would remove the feature to have minimal changes to the mesh, e.g. be able to only change the "points" file of the mesh. My guess here is that either a debug flag could be put in place for reporting these small details or use a function object for reporting where the current mesh files came from. If you prefer/concur, I can create said function object. |
|
|
I am still running tests on the various changes to OpenFOAM-dev and I haven't yet completed the changes corresponding to patch-1 in a way I am entirely happy with. I hope to get this done by the end of the day. > enforce that the core files for the mesh ("faces", "points", etc...) should all come from the same time folder... I don't agree with this change. > which would remove the feature to have minimal changes to the mesh, e.g. be able to only change the "points" file of the mesh This is a very useful feature which we want to keep. I am not convinced of any need to change the way the mesh is constructed or reported, I think it will introduce more complexity requiring maintenance. If you created a functionObject for this do you think users will actually use it? |
|
|
> I hope to get this done by the end of the day. OK, then I'll do some more tests later on and report back with another bug report, if the need arises. > If you created a functionObject for this do you think users will actually use it? Good point. Advanced users might use it, but many of those advanced users can create and use scripts to pinpoint where those files are coming from. Really advanced users simply know where their meshes are located! Any other users would only use it if they see it in the Doxygen documentation and/or if they see it somewhere already explaining how to use it. Very well, if the need ever arises with a good enough reason, this can be easily done at that time. In fact, this is one those features simple enough to be used for training users in the art of creating function objects :) Or even with "codedFunctionObject"! |
|
|
I am playing with patch-1 still and wondering if sqrt(SMALL) is the appropriate tolerance. It doesn't work with SMALL; maybe it should relate to some fraction of deltaT_. What is the rationale for sqrt(SMALL)? Do you think this will be robust? |
|
|
How about 10*SMALL? My feeling is that sqrt(SMALL) will cause problems for cases with very small time-steps. |
|
|
Sorry, I should have detailed this beforehand. I originally was using "mag(deltaT*1.001)" vs "mag(deltaT)", but it felt like too much of a hard-coded hack, hence switching to something based on "SMALL". The logic behind "sqrt(SMALL)" is that an error of roughly 1e-7 for something that should be "0.0" felt like it was "good enough". But a margin of error of "deltaT*1.001" is probably the easiest/best approximation. Since the idea is to confirm that the added "deltaT" value worked as intended; the problem is that the margin of error should be near machine epsilon, but the addition of errors in the operations makes it something that could be in the order of 10 to 1000 times larger than machine epsilon, due to scalar->word->scalar operations :( |
|
|
This tolerance certainly relates to machine precision but my test show the issue is simply to do with the subtractions not the scalar->word->scalar operations. I have just pushed the changes with 100*SMALL as the tolerance. See if you can break it :-) |
|
|
The check for "this still needs to be fixed" was: mag(rereadTimeValue - oldTimeValue) > mag(deltaT_*1.001) But then in v1 I provided the "simpler" form: mag(rereadTimeValue - oldTimeValue - deltaT_) > sqrt(SMALL) If the margin of error is in the same scale as "deltaT_" itself, then things are seriously not working as intended... Forgot to also say that "10*SMALL" didn't work for me. |
|
|
> See if you can break it Broke it twice in the same run :D (dev commit 30bbd3e9) Time = 99.96125 --> FOAM Warning : From function Time::operator++() in file db/Time/Time.C at line 1123 Increased the timePrecision from 7 to 15 to distinguish between timeNames at time 99.9613000000001 Time = 99.9613000000001 ... Time = 99.9999500000014 --> FOAM Warning : From function Time::operator++() in file db/Time/Time.C at line 1123 Increased the timePrecision from 15 to 16 to distinguish between timeNames at time 100.0000000000014 Time = 100.0000000000014 |
|
|
What is the smallest tolerance for which the case runs? |
|
|
In this test case, it seems "10000*SMALL" is safe enough. I'll do some more tests, to try and isolate what could be the origin of this issue... |
|
|
For DP 10000*SMALL would probably be OK but in SP it would be nonsense. I think we do need to find out why it is accumulating SOOO much error. |
|
|
I'm looking into the issue regarding the error level... it's weird what I'm seeing right now... edit: Sorry... the weird detail is that it seemed that "deltaT" didn't even add up to "value()"... |
|
|
|
|
|
OK, attached is the file "issue815.patch.v3", which is relative to OpenFOAM-dev commit 30bbd3e9be9d. The changes are (in order): 1- Initialized "timeNameValue", because nothing worse than using memory garbage in case the "readScalar" fails due to some crazy reason... although I didn't check of "readScalar" already does this... 2- "errorMargin" is a new variable that has the calculation that takes into account: a) the current expected numerical precision; b) the "deltaT" value; c) and the maximum allowed precision (i.e. SMALL). This is because the minimum precision needed should be the one currently being used, along with the current "deltaT"... and because we cannot allow it to go over machine epsilon, we should limit the expected numerical difference. 3- "do-while" loop has a massive flaw of always increasing the precision. My tests could reach over 7700 precision... So I replaced with a "for" loop. ... Then again, not sure if this was the best solution... probably should have used "if(precision_<maxPrecision_)" for wrapping the "do-while" loop... 4- With the replacement of the "do-while" loop for the "for" loop, the warning should only be issued if the precision has actually changed. |
|
|
1. I took out the initialization because in case the read fails false is returned and the value is not used. How does initializing to -VGREAT in particular help? Why not 0 or SMALL or anything else? 3. Given that we are looping to a maximum I prefer a for-loop anyway as I used in the code added to setControls. I will merge in and test. |
|
|
Isn't for ( /* no init */ ; precision_ < maxPrecision_ && readScalar(dimensionedScalar::name().c_str(), timeNameValue) && (mag(timeNameValue - oldTimeValue - deltaT_) > errorMargin) ; /* no iter */ ) simply a while-loop? |
|
|
The "for" loop by concept will always check the middle statement at the start of the loop. The "while" loop.... Oh, good point! Yes it is! Completely forgot that "while" loop exists in C/C++... I was assuming that "do-while" was the only one similar loop... My experience in coding is that initializing values is always a must, since we never know for certain who's going to be messing with the code next; or even the original coder can forget how the variable was used in the first place. And the usual result is having to track down a bug that only occurs in release mode, because of uninitialised memory. I chose "-VGREAT" since it would give the biggest difference in 99.99999% of the cases... except for negative pseudo-temporal time values (e.g. crank degree?). |
|
|
Just pushed the latest... |
|
|
Tested it just now. OK, this is.... irgh... Using the test package "runTimeSteps.tar.gz" I provided before, if we run the test case until its end, it will write the last step at "99.958". Now, if we up the "deltaT" from "5.0e-4" to "0.5432112345e-5", the current algorithm is not able to provide a fully accurate precision right off the bat: Create mesh for time = 99.958 ... Increased the timePrecision from 5 to 8 to distinguish between timeNames at time 99.958005 Time = 99.958005 ... Increased the timePrecision from 8 to 10 to distinguish between timeNames at time 99.95801086 Time = 99.95801086 ... Increased the timePrecision from 10 to 12 to distinguish between timeNames at time 99.9580162963 Time = 99.9580162963 ... Increased the timePrecision from 12 to 13 to distinguish between timeNames at time 99.95802172845 Time = 99.95802172845 ... Increased the timePrecision from 13 to 15 to distinguish between timeNames at time 99.9580271605617 Time = 99.9580271605617 ... Increased the timePrecision from 15 to 16 to distinguish between timeNames at time 99.95803259267407 Time = 99.95803259267407 Then later on: Time = 99.99999566054473 ... Increased the timePrecision from 16 to 17 to distinguish between timeNames at time 100.00000109265707 Time = 100.00000109265707 ExecutionTime = 0.41 s ClockTime = 0 s This is because the "timeTol" is not updated whenever the precision is increased as well. Problem is that when I tried this, it didn't go so well. In addition, I'm not able to do any more tests today, sorry :( With luck, I can do some tomorrow at the end of the day. |
|
|
OK, I've finally managed to get a better look into this, but unfortunately when I reach Saturdays, my head doesn't run close enough to "optimal performance", therefore I can't figure out the "optimum solution"... Anyway, what I can figure out is this: 1- The current implementation seems to cover most of the cases, although not all of them. It progressively increases precision, in an attempt to not go "overkill" at the first confirmation that the precision needs updating. The only "blind spot" I can think of, with the current solution, is that the rounding algorithm can lead to a corner case situation conceptually similar to this one: Time step: 0.123456 Delta time step: 0.0000005 Time step: 0.1234565 Delta time step: 0.000000048 (adapted from CourantNo) Time step: 0.1234566 (real 0.123456548) Delta time step: 0.000000008 (adapted from CourantNo) Time step: 0.123456556 (real 0.123456556) This isn't very realistic, since this seems to be covered by the min-max with "deltaT" and "pow(10,-precision)"... but it's the concept that matters here, namely the situation where a round-off could potentially break the time flow. I think that for now, this can be at least automatically detected and issue a warning to the user... at least for the current development. The idea is to do a check on whether the saved time folder didn't go in the wrong direction. If I'm not 100% certain about this, but from my tests, this seems to work as intended: //Check if there was a round-off glitch that reversed time scalar oldTimeNameValue = -VGREAT; if ( readScalar(oldTimeName.c_str(), oldTimeNameValue) && (sign(timeNameValue - oldTimeNameValue) != sign(deltaT_)) ) { WarningIn("Time::operator++()") << "Current time name " << dimensionedScalar::name() << " is set to an instance prior to the previous one " << oldTimeName << endl << " This might result in temporal discontinuities." << endl; } This would theoretically circumvent all of the corner cases, at least the ones I can think of :( 2- The next issue is regarding the recovery of a time snapshot that was saved with "enhanced precision". I'm referring to the "value" entry stored in the "uniform/time" file, for example in "99.9580001/uniform/time.value". Said "value" doesn't save with the same precision as the time folder, which according to the comments in the methods that use this value, seems to be the desired effect. The suggestion here would be to have at least one accurate way of storing the time data in binary format. This way it would be possible to reconstruct the time folders accurately, if the need ever arises. Said time reconstruction feature could be introduced in "foamListTimes" as an additional option... or at the very least, display the actual time snapshots each folder is referring to, so that the user fixes manually said folders. |
|
|
The check you added for issue 1 is not clear to me yet, I need to think about and test it. Where did you insert the test for change in time direction? Issue 2 is clear and my feeling is that the time value should be stored in uniform/time with the same precision as the time directory i.e. we should write the timeName as a value. The alternatives would be to write it binary (I don't like this one at all) or write it ASCII but with sufficient precision to provide an accurate read. I think writing timeName as the value would be a good approach and I can't immediately see any issues with this. |
|
|
Sorry, completely forgot about where to place the check for #1... I think it should be tested after this if-block in the method "Foam::Time::operator++()": if (precision_ == maxPrecision_) { // Reached maxPrecision limit WarningIn("Time::operator++()") << "Current time name " << dimensionedScalar::name() << " is the old as the previous one " << oldTimeName << endl << " This might result in overwriting old results." << endl; } As for your solution to #2: seems like a good solution. Precision "log10(SMALL)+1" should ensure the value to be properly stored in ASCII format; the extra digit would be an attempt to ensure nothing is missing from the number, in comparison to binary. |
|
|
I have just pushed these two changes to OpenFOAM-dev |
|
|
I am finding the new precision alteration code unreliable for cases with adjustable time-step, for example running tutorials/multiphase/twoPhaseEulerFoam/laminar/mixerVessel2D I get time directories; 3.9 4 4.1000000000000005 4.2000000000000002 4.2999999999999998 4.4000000000000004 4.5 |
|
|
Sorry, I found about this late last night. I didn't report right away, because I wanted to figure out first what's going wrong :( I found out when I re-ran my test case yesterday from "0", instead from 99.958 and found the same symptom, for example: - Settings: startTime 0; stopAt endTime; endTime 100; deltaT 0.5e-5; writeControl runTime; writeInterval 0.54325; purgeWrite 0; writeFormat ascii; writePrecision 6; writeCompression off; timeFormat general; timePrecision 2; - Snippet of the first major problem: Time = 49.146475 --> FOAM Warning : From function Time::operator++() in file db/Time/Time.C at line 1127 Increased the timePrecision from 8 to 10 to distinguish between timeNames at time 49.14648001 Time = 49.14648001 Unfortunately, I won't be able to diagnose this in more detail today. Nonetheless, I expect I'll be able to do so next weekend. Sooner if I can deduce what's going on. Nonetheless, from the numbers I checked yesterday from comment #4056, it looked like one of the critical problems was the reliance on the updated precision, even though it's only done at the next "deltaT" update. For example, when the precision went from 1e-8 to 1e-10, it was already overkill, although it was necessary due to the difference in "t2-t1-dT" being at a scale of "Xe-9" that was larger than "1e-9". At the time I simply accepted it as a "good thing" and was why I reported on comment #4120: "The current implementation seems to cover most of the cases [...]" This feels like something that is already solved somewhere in the programming scientific community, but I haven't managed to look for it yet either. I'll let you know if I find out something in the meantime. |
|
|
I have just pushed this change: commit 7dfdea2cac3c955a174168d3d7671a57cc3ece19 Author: Henry <Henry> Date: Sun Mar 15 18:49:52 2015 +0000 Time: Adjust the precision of the time-directories only for write-times Avoids unnecessary increases in precision during intermediate steps for cases with adjustable time-step which may require very high precision to represent the time name. My feeling is that now that the precision adjustment guarantees sufficient precision irrespective of the time and time-step, for cases in which the time-step is adjusted this precision adjustment can ONLY be applied for output times otherwise it will rapidly increase to the upper limit. The only issue if for cases in which intermediate results are written explicitly for any time. For such cases the precision must be set by the user appropriately. |
|
|
I've finished doing a few more tests with the latest commit and it does seem to work very well! Or at the very least, it won't reveal the issues we've seen during the development of this feature, unless the user is writing every single time step, which is unlikely in a non-debug-type simulation... in which situation of a debug, the user should have increased the precision to maximum before even starting the run :) Although it does look weird, the solver showing several identical time steps... but it's the user's fault anyway :) Nonetheless, I was able to trigger a curious bug/detail... here's the message I got: Time = 128.20623875389433 --> FOAM Warning : From function Time::operator++() in file db/Time/Time.C at line 1302 Increased the timePrecision from 17 to 18 to distinguish between timeNames at time 128.206781965128812 --> FOAM Warning : From function Time::operator++() in file db/Time/Time.C at line 1312 Current time name 128.206781965128812 is the old as the previous one 128.20623875389433 This might result in overwriting old results. Time = 128.206781965128812 So essentially the problem is the second warning, which is triggered when "precision_ == maxPrecision_", which reveals two issues: 1- The sentence: "Current time name ... is the old as the previous one ..." sounds strange. Maybe it was meant to be: "Current time name ... is the same as the previous one ..." 2- Uh... assuming the correction is right, then the sentence isn't a valid statement anyway, since the values are clearly different. It seems to me that the correct statement would be something like: "The maximum time precision has been reached. This is the last warning regarding time precision you'll see during this run." In which case, the statement below that is still valid: " This might result in overwriting old results." Beyond this last textual adjustment, I think it's good to go! We've already covered so many situations, that the last resort would be to either go Quad Precision or go "binary" for those who want perfect time precision, in which case, it would probably require a more dedicated data storage handling mechanism. |
|
|
I think we have reached a point where users can test the new functionality in anger and provide feedback. I have changed the max-precision messages as you suggested. Resolved by commit fbb0a87995dc41c2183a1bc2648790352902d677 in OpenFOAM-dev |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2013-04-15 08:32 | projectionist | New Issue | |
| 2013-04-15 08:32 | projectionist | File Added: controlDict | |
| 2013-07-07 19:05 |
|
Note Added: 0002310 | |
| 2013-07-08 10:04 |
|
Note Added: 0002311 | |
| 2015-03-06 14:22 | wyldckat | Note Added: 0003992 | |
| 2015-03-07 19:58 | wyldckat | File Added: runTimeSteps.tar.gz | |
| 2015-03-07 20:12 | wyldckat | Note Added: 0004003 | |
| 2015-03-07 21:35 | wyldckat | File Added: Time.C.v1 | |
| 2015-03-07 21:35 | wyldckat | File Added: issue815.patch.v1 | |
| 2015-03-07 21:47 | wyldckat | Note Added: 0004004 | |
| 2015-03-07 22:02 | henry | Note Added: 0004006 | |
| 2015-03-07 22:20 | wyldckat | Note Added: 0004007 | |
| 2015-03-07 22:43 | wyldckat | Note Added: 0004008 | |
| 2015-03-07 22:43 | wyldckat | File Added: issue815.patch.v2 | |
| 2015-03-07 23:53 | henry | Note Added: 0004010 | |
| 2015-03-08 10:50 | henry | Note Added: 0004011 | |
| 2015-03-08 11:42 | wyldckat | Note Added: 0004012 | |
| 2015-03-08 11:51 | henry | Note Added: 0004013 | |
| 2015-03-08 11:52 | wyldckat | Note Added: 0004014 | |
| 2015-03-08 11:57 | henry | Note Added: 0004015 | |
| 2015-03-08 12:04 | wyldckat | Note Added: 0004016 | |
| 2015-03-08 12:07 | henry | Note Added: 0004017 | |
| 2015-03-08 12:11 | wyldckat | Note Added: 0004018 | |
| 2015-03-08 12:14 | henry | Note Added: 0004019 | |
| 2015-03-08 12:21 | wyldckat | Note Added: 0004020 | |
| 2015-03-08 12:22 | wyldckat | Note Edited: 0004020 | |
| 2015-03-08 12:24 | henry | Note Added: 0004021 | |
| 2015-03-08 12:27 | wyldckat | Note Added: 0004022 | |
| 2015-03-08 12:30 | henry | Note Added: 0004023 | |
| 2015-03-08 14:53 | wyldckat | Note Added: 0004024 | |
| 2015-03-08 14:54 | wyldckat | Note Edited: 0004024 | |
| 2015-03-08 15:03 | henry | Note Added: 0004025 | |
| 2015-03-08 15:17 | wyldckat | Note Added: 0004026 | |
| 2015-03-08 15:55 | henry | Note Added: 0004027 | |
| 2015-03-08 16:17 | henry | Note Added: 0004029 | |
| 2015-03-08 16:31 | wyldckat | Note Added: 0004030 | |
| 2015-03-08 16:35 | henry | Note Added: 0004031 | |
| 2015-03-08 16:36 | wyldckat | Note Added: 0004032 | |
| 2015-03-08 17:14 | wyldckat | Note Added: 0004038 | |
| 2015-03-08 17:14 | wyldckat | Note Edited: 0004038 | |
| 2015-03-08 17:57 | henry | Note Added: 0004041 | |
| 2015-03-08 18:27 | wyldckat | Note Added: 0004042 | |
| 2015-03-08 18:32 | henry | Note Added: 0004043 | |
| 2015-03-08 19:07 | wyldckat | Note Added: 0004044 | |
| 2015-03-08 19:09 | wyldckat | Note Edited: 0004044 | |
| 2015-03-08 20:01 | wyldckat | File Added: issue815.patch.v3 | |
| 2015-03-08 20:14 | wyldckat | Note Added: 0004046 | |
| 2015-03-08 20:37 | henry | Note Added: 0004047 | |
| 2015-03-08 20:42 | henry | Note Added: 0004048 | |
| 2015-03-08 20:56 | wyldckat | Note Added: 0004051 | |
| 2015-03-08 21:10 | henry | Note Added: 0004055 | |
| 2015-03-08 21:27 | wyldckat | Note Added: 0004056 | |
| 2015-03-14 14:04 | wyldckat | Note Added: 0004120 | |
| 2015-03-14 14:33 | henry | Note Added: 0004121 | |
| 2015-03-14 14:42 | wyldckat | Note Added: 0004123 | |
| 2015-03-14 20:36 | henry | Note Added: 0004127 | |
| 2015-03-15 10:19 | henry | Note Added: 0004130 | |
| 2015-03-15 12:22 | wyldckat | Note Added: 0004131 | |
| 2015-03-15 18:56 | henry | Note Added: 0004135 | |
| 2015-03-22 00:20 | wyldckat | Note Added: 0004181 | |
| 2015-03-22 09:52 | henry | Note Added: 0004183 | |
| 2015-03-22 09:52 | henry | Status | new => resolved |
| 2015-03-22 09:52 | henry | Resolution | open => fixed |
| 2015-03-22 09:52 | henry | Assigned To | => henry |
| 2015-03-24 00:17 | liuhuafei | Issue cloned: 0001589 |
