View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001853 | OpenFOAM | Bug | public | 2015-09-17 11:43 | 2015-09-17 22:09 |
| Reporter | chriss85 | Assigned To | henry | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Platform | GNU/Linux | OS | Other | OS Version | (please specify) |
| Summary | 0001853: pairPatchAgglomeration doesn't call compactLevels() | ||||
| Description | I was trying to use pairPatchAgglomeration to create a coarser boundary mesh and I noticed that the class doesn't call compactLevels after agglomeration, to properly resize its lists to the actual count of agglomeration levels. This seems to make it impossible to loop over the levels without knowing the amount of valid levels and is probably not intended. This function should be called at the end of agglomerate(). | ||||
| Tags | No tags attached. | ||||
|
|
By the way, OpenFOAM currently doesn't provide any algorithm to create a coarser mesh in a similar fashion to pairPatchAgglomeration? |
|
|
It is not clear what changes you require or what the consequence would be. Have you implemented and tested the change you are asking for? Currently compactLevels is called as required in dummyAgglomeration.C pairGAMGAgglomerate.C MGridGenGAMGAgglomeration.C > By the way, OpenFOAM currently doesn't provide any algorithm to create a coarser mesh in a similar fashion to pairPatchAgglomeration Correct |
|
|
Sorry if anything is unclear. I was talking about calling pairPatchAgglomeration::compactLevels() at the end of pairPatchAgglomeration::agglomerate(), in a similar fashion as in the files you mention, so that the patchLevels_ and other lists in the class are correctly sized after patch agglomeration. Please correct me if I have misunderstood something, but in my application it was necessary to add this call to be able to request the number of agglomerated levels from the class after agglomerate() was called. Before adding this call, the list sizes are still at the maximum size and it would've been necessary to check the pointer validity since the list could contain null entries. After calling the function at the end of agglomerate() the lists are correctly sized and it is safe to access their elements. |
|
|
Currently the call to compactLevels is made after the call to agglomerate in case other processing is necessary before the levels are compacted. I do not have time right now to work through the implications of moving the call to compactLevels to the end of agglomerate. Have you tested this change? Have you checked through the operation of the code which calls agglomerate and then compactLevels to make sure it is OK to move the compactLevels call into agglomerate? |
|
|
The thing is that the compactLevels function is a private one in pairPatchAgglomeration so it cannot be called from elsewhere anyway. The only piece of code I have found that uses the pairPatchAgglomeration class is the faceAgglomerate tool used for viewFactor radiation. This tool doesn't access the patchLevels list in a way that would make the call of compactLevels() necessary, so no official code would be affected by this change. If the function is called, the unused levels are simply removed from these lists so the result is somewhat more clean. faceAgglomerate only accesses a list that is already initialized with the correct size in agglomerate() directly. I agree that this isn't a big issue but I initially had some problems here because I was not expecting that the list with the agglomerated patch levels would contain invalid results after agglomeration. A simple call of this function fixes this, so it should be safe to add it. The only line I have added at the end of pairPatchAgglomeration::agglomerate() is compactLevels(nCreatedLevels); |
|
|
I agree that either compactLevels needs to be called by agglomerate or it has to be public so it can be called at a higher level. I have made this change in OpenFOAM-dev: commit 9f666b0dbd563f7a79855318fabc267f9f70b7f3 and will put it on test. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2015-09-17 11:43 | chriss85 | New Issue | |
| 2015-09-17 11:45 | chriss85 | Note Added: 0005363 | |
| 2015-09-17 12:47 | henry | Note Added: 0005364 | |
| 2015-09-17 14:14 | chriss85 | Note Added: 0005365 | |
| 2015-09-17 14:17 | chriss85 | Note Edited: 0005365 | |
| 2015-09-17 16:07 | henry | Note Added: 0005366 | |
| 2015-09-17 16:28 | henry | Note Edited: 0005366 | |
| 2015-09-17 16:29 | chriss85 | Note Added: 0005367 | |
| 2015-09-17 16:32 | chriss85 | Note Edited: 0005367 | |
| 2015-09-17 17:07 | henry | Note Added: 0005368 | |
| 2015-09-17 17:07 | henry | Status | new => resolved |
| 2015-09-17 17:07 | henry | Resolution | open => fixed |
| 2015-09-17 17:07 | henry | Assigned To | => henry |
