View Issue Details

IDProjectCategoryView StatusLast Update
0003392OpenFOAMFeaturepublic2019-11-20 13:07
ReporterblttkglAssigned Tohenry 
PrioritylowSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformGNU/LinuxOSUbuntuOS Version15.04
Product Versiondev 
Fixed in Versiondev 
Summary0003392: ISAT statistics are misleading
DescriptionIn TDACChemistryModel, the cpu time statistics generated for ISAT are a bit misleading.

Algorithm goes so that for each cell:

* If retrieve is successful, time it takes is written to cpu_retrieve
* If retrieve returns false, chemistry is solved and cpu time written to cpu_solve
* Then this solved chemistry is either added to the binary tree, or used to grow an existing binary tree. Time it takes for this operation is written to cpu_add or cpu_grow.

Currently, cpu_add and cpu_grow also includes to time for solving the chemistry cell, so cpu_add = addTime+ solveTime and cpu_grow = growTime + solveTime. When processing the statistics, especially for a parallel job where the user is trying to locate the bottleneck rank for each individual operation, this may be misleading.

Of course one can simply substract cpu_solve from cpu_add and cpu_grow for each time step, but I think cpu_add and cpu_grow should only reflect the time it takes to perform those individual operations.
Steps To ReproduceIn OpenFOAM-dev, in OpenFOAM-dev/src/thermophysicalModels/chemistryModel/chemistryModel/TDACChemistryModel/TDACChemistryModel.C , check line 697 to 734

TagsNo tags attached.

Activities

fcontino

2019-11-20 12:27

reporter   ~0010924

This is an excellent point. The statistics would be easier to understand without accumulation in the different times. I have adjusted the code accordingly. Could you please check if it works as you think is more efficient? (see uploaded file)

TDACChemistryModel.C (23,390 bytes)

blttkgl

2019-11-20 12:36

reporter   ~0010925

Exactly, timeTmp (solve time) is removed from addNewLeafCpuTime and growCpuTime. We are using the ISAT logs like this and it makes more sense when benchmarking the performance.


Bulut

henry

2019-11-20 12:51

manager   ~0010927

Resolved by commit f66ea315840b4e2bfb86f522a9467adef89562a9

Issue History

Date Modified Username Field Change
2019-11-20 08:24 blttkgl New Issue
2019-11-20 12:27 fcontino File Added: TDACChemistryModel.C
2019-11-20 12:27 fcontino Note Added: 0010924
2019-11-20 12:36 blttkgl Note Added: 0010925
2019-11-20 12:50 henry Category Bug => Feature
2019-11-20 12:51 henry Assigned To => henry
2019-11-20 12:51 henry Status new => resolved
2019-11-20 12:51 henry Resolution open => fixed
2019-11-20 12:51 henry Fixed in Version => dev
2019-11-20 12:51 henry Note Added: 0010927