Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restructure grid_movement and adt_structure, remove SU2_MSH and grid_adaptation #1035

Merged
merged 12 commits into from
Sep 5, 2020

Conversation

jayantmukho
Copy link
Contributor

Proposed Changes

This PR breaks up the following files:

  1. grid_movement_structure
  2. adt_structure

It also gets rid of grid_adaptation_structure and the entire SU2_MSH module. These files have not be updated in a long time are are deprecated. None of the new work in mesh adaptation by @bmunguia uses any of these routines.

Related Work

#583

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with the '-Wall -Wextra -Wno-unused-parameter -Wno-empty-body' compiler flags).
  • My contribution is commented and consistent with SU2 style.
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp) , if necessary.

};
#include "./CADTBaseClass.hpp"
#include "./CBBoxTargetClass.hpp"
#include "../omp_structure.hpp"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was quite some room for cutting down includes. I tried to include the bare minimum in the new files

* \brief Class for storing the information needed in a node of an ADT.
* \author E. van der Weide
*/
struct CADTNodeClass {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file, and the CBBoxTargetClass only contain structs but I still split them into their own header files. These don't have any cpp files associated with them

if(guaranteedMinDist2 > other.guaranteedMinDist2) return false;
if(boundingBoxID < other.boundingBoxID) return true;
return false;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the functions here are inline, so there isn't a corresponding cpp file

@@ -102,9 +102,6 @@ AC_ARG_ENABLE(CFD,
AC_ARG_ENABLE(DOT,
AS_HELP_STRING([--disable-DOT], [build the SU2_DOT executable (default = yes)]),
[build_DOT=$enableval], [build_DOT="yes"])
AC_ARG_ENABLE(MSH,
AS_HELP_STRING([--disable-MSH], [build the SU2_MSH executable (default = yes)]),
[build_MSH=$enableval], [build_MSH="yes"])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't a 100% sure I needed to do to this configure file. I just searched for MSH and deleted any offending lines. Let me know if something else needs to be done

Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup 👍

SU2_CFD/include/solvers/CFEASolver.hpp Show resolved Hide resolved
Common/src/grid_movement/CGridMovement.cpp Outdated Show resolved Hide resolved
Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were saying yesterday that since this removes functionality maybe it should go with 7.1 (currently expected for end of July but possibly August) but waiting is going to give you some conflicts to solve (version number changes etc.) so it is up to you...

@jayantmukho
Copy link
Contributor Author

I don't mind waiting. Most of these files aren't usually changed by PRs. I'll keep this up to date.

@pcarruscag
Copy link
Member

They will be changed when the version number changes to 7.0.6, and git will tell you "deleted by us, modified by them" just a heads up.

@stale
Copy link

stale bot commented Sep 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still a relevant issue please comment on it to restart the discussion. Thank you for your contributions.

@stale stale bot added the stale label Sep 4, 2020
@stale stale bot removed the stale label Sep 5, 2020
@pcarruscag pcarruscag merged commit 0166ba0 into develop Sep 5, 2020
@pcarruscag pcarruscag deleted the restructure_grid_movement branch September 5, 2020 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants