Cleanup and document the multigrid agglomeration strategy#1372
Cleanup and document the multigrid agglomeration strategy#1372pcarruscag merged 7 commits intodevelopfrom
Conversation
|
This pull request fixes 7 alerts when merging cb9fce3 into 5ce84cc - view on LGTM.com fixed alerts:
|
|
This pull request fixes 7 alerts when merging c4f27dc into 6338bff - view on LGTM.com fixed alerts:
|
WallyMaier
left a comment
WallyMaier
left a comment
There was a problem hiding this comment.
LGTM! Thanks for taking the time for cleaning up the code and removing the redundancy!
TobiKattmann
left a comment
TobiKattmann
left a comment
There was a problem hiding this comment.
Thanks for this cleanup 💐
| /*! | ||
| * \brief Constructor of the class. | ||
| */ | ||
| CGeometry(void); |
There was a problem hiding this comment.
Fun Fact, there are 1475 (void) in SU2_CFD and Common ... We/I could make them go extinct (if we are ok with some possible minor merge conflicts in other peoples PR's )
| */ | ||
| CMultiGridGeometry(CGeometry **geometry, CConfig *config_container, unsigned short iMesh); | ||
|
|
||
| private: |
There was a problem hiding this comment.
👍 looks like all methods were public before
| * \param[in] config - Definition of the particular problem. | ||
| */ | ||
| bool GeometricalCheck(unsigned long iPoint, CGeometry *fine_grid, CConfig *config); | ||
| bool GeometricalCheck(unsigned long iPoint, const CGeometry *fine_grid, const CConfig *config) const; |
There was a problem hiding this comment.
Maybe with that amount of changes clang-format could have been a thing. (Again pointer-Alignment is the rat here)
| unsigned long iPoint, Index_CoarseCV, iElem, iVertex, iteration, nVertexS, nVertexR, | ||
| nBufferS_Vector, nBufferR_Vector, iParent, jVertex,Local_nPointCoarse, Local_nPointFine, Global_nPointCoarse, Global_nPointFine, | ||
| *Buffer_Receive_Parent = nullptr, *Buffer_Send_Parent = nullptr, *Buffer_Receive_Children = nullptr, *Buffer_Send_Children = nullptr, | ||
| *Parent_Remote = nullptr, *Children_Remote = nullptr, *Parent_Local = nullptr, *Children_Local = nullptr; | ||
| short marker_seed; | ||
| bool agglomerate_seed = true; | ||
| unsigned short nChildren, iNode, counter, iMarker, jMarker, priority, MarkerS, MarkerR, *nChildren_MPI; | ||
| vector<unsigned long> Suitable_Indirect_Neighbors, Aux_Parent; | ||
| vector<unsigned long>::iterator it; | ||
|
|
||
| unsigned short nMarker_Max = config->GetnMarker_Max(); | ||
|
|
||
| unsigned short *copy_marker = new unsigned short [nMarker_Max]; | ||
|
|
||
| #ifdef HAVE_MPI | ||
| int send_to, receive_from; | ||
| SU2_MPI::Status status; | ||
| #endif |
| // unsigned long iPointFree = nPointDomain-1; | ||
| // iCoarsePoint = 0; | ||
| // | ||
| // do { |
There was a problem hiding this comment.
haven't seen a do - while in quite some time.
Good to get rid of those commented code blocks
| wall_temperature.fine_grid = fine_grid; | ||
| wall_temperature.marker = val_marker; | ||
| wall_temperature.target = CustomBoundaryTemperature[val_marker]; | ||
|
|
||
| SetMultiGridWallQuantity(fine_grid, val_marker, wall_temperature); |
There was a problem hiding this comment.
Nice cut-paste-specialize
3 participants
Proposed Changes
In trying to understand the algorithm a little better I also tried to improve the documentation and clean up some redundancy.
PR Checklist