View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003392 | OpenFOAM | Feature | public | 2019-11-20 08:24 | 2019-11-20 13:07 |
| Reporter | blttkgl | Assigned To | henry | ||
| Priority | low | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Platform | GNU/Linux | OS | Ubuntu | OS Version | 15.04 |
| Product Version | dev | ||||
| Fixed in Version | dev | ||||
| Summary | 0003392: ISAT statistics are misleading | ||||
| Description | In 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 Reproduce | In OpenFOAM-dev, in OpenFOAM-dev/src/thermophysicalModels/chemistryModel/chemistryModel/TDACChemistryModel/TDACChemistryModel.C , check line 697 to 734 | ||||
| Tags | No tags attached. | ||||
|
|
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) |
|
|
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 |
|
|
Resolved by commit f66ea315840b4e2bfb86f522a9467adef89562a9 |
| 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 |
