Migrate include after ifdefs in core and solver h #1170

blaisb · 2024-06-12T18:41:28Z

Description

Everywhere throughout the include files of the core and the solver library, there were multiple includes that were defined before the #ifndef -> #define -> #endif statements. This is mostly aesthetics, but this can also have a small impact on compilation time, especially on systems with a slow filesystem (e.g. the clusters). See for example: https://stackoverflow.com/questions/20988705/is-an-include-before-ifdef-define-include-guard-okay

Additionally, some of the include guards were in capital letters, some were not. Heck, some were even missing. So I took the opportunity to uniformize thing. I limited myself to core/solver as a first step, but in a follow-up PR I could do it for the other parts of the library.

Testing

There are not tests required here. If the code compiles, everything will be fine.

Documentation

No documentation was modified in the making of this PR.

Checklist (will be removed when merged)

See this page for more information about the pull request process.

Code related list:

All in-code documentation related to this PR is up to date (Doxygen format)
Lethe documentation is up to date
The branch is rebased onto master
Code is indented with indent-all and .prm files (examples and tests) with prm-indent

Pull request related list:

No other PR is open related to this refactoring
Labels are applied
There are at least 2 reviewers (or 1 if small feature) excluding the responsible for the merge
If this PR closes an issue or is related to a project, it is linked in the "Projects" or "Development" section
If any future works is planed, an issue is opened
The PR description is cleaned and ready for merge

acdaigneault

I like those small clean up.
It's just small questions in my comments on the way we should name things, but I don't think changes would be required in this respect.

include/core/solid_objects_parameters.h

include/core/tensors_and_points_dimension_manipulation.h

include/solvers/copy_data.h

include/core/auxiliary_math_functions.h

acdaigneault · 2024-06-12T19:01:44Z

include/solvers/postprocessors_smoothing.h

@@ -1,3 +1,18 @@
+/* ---------------------------------------------------------------------
+ *
+ * Copyright (C) 2019 -  by the Lethe authors


Do you want to update the year?

Maybe in another PR when we change the license everywhere

Co-authored-by: Audrey Collard-Daigneault <71884806+acdaigneault@users.noreply.github.com>

AmishgaAlphonius

Nice uniformization and it's cool to learn that it can affect compile time.
I left a few comments and some files were not modified, but they do not follow the exact same format as the others. I've listed them here:

In include/core/inexact_newton_non_linear_solver.h, it should be lethe_inexact_newton_non_linear_solver_h instead of lethe_inexact_newton_iteration_non_linear_solver_h
In include/core/kinsol_newton_non_linear_solver.h, it should be lethe_kinsol_newton_non_linear_solver_h instead of kinsol_non_linear_solver_h
In include/core/phase_change.h, it should be lethe_phase_change_h instead of phase_change_h
In include/solvers/cahn_hilliard.h, it should be lethe_cahn_hilliard_h instead of cahn_hilliard_h

include/core/dem_properties.h

AmishgaAlphonius · 2024-06-12T20:15:02Z

include/core/lethe_grid_tools.h

@@ -18,7 +18,6 @@



lines 16-17, it should be lethe_grid_tools_h instead of lethe_lethegridtools_h

should actually be lethe_lethe_grid_tools_h
This class is weirdly named, will be refactored :)

include/core/mesh_controller.h

include/solvers/postprocessors.h

include/solvers/postprocessors_smoothing.h

include/solvers/simulation_parameters.h

Co-authored-by: Amishga Alphonius <107414376+AmishgaAlphonius@users.noreply.github.com>

acdaigneault · 2024-06-12T22:05:05Z

Btw, thank you for the super exhaustive PR description :D

blaisb · 2024-06-12T22:08:57Z

Btw, thank you for the super exhaustive PR description :D

Well rumor has it that someone has made a super cool amazing templates for PR and it makes it super easy now to write good PR messages. I'm just following the new very clear guidelines.

@brief

Description Quick refactor as done in PR #1170 that moves header includes after the #define. Some @brief has been added here and there at the same time.

Description Everywhere throughout the include files of the core and the solver library, there were multiple includes that were defined before the #ifndef -> #define -> #endif statements. This is mostly aesthetics, but this can also have a small impact on compilation time, especially on systems with a slow filesystem (e.g. the clusters). See for example: https://stackoverflow.com/questions/20988705/is-an-include-before-ifdef-define-include-guard-okay Additionally, some of the include guards were in capital letters, some were not. Heck, some were even missing. So I took the opportunity to uniformize thing. I limited myself to core/solver as a first step, but in a follow-up PR I could do it for the other parts of the library. Testing There are not tests required here. If the code compiles, everything will be fine. Documentation No documentation was modified in the making of this PR. Co-authored-by: Audrey Collard-Daigneault <71884806+acdaigneault@users.noreply.github.com> Co-authored-by: Amishga Alphonius <107414376+AmishgaAlphonius@users.noreply.github.com> Former-commit-id: eea049f

@brief

Description Quick refactor as done in PR chaos-polymtl#1170 that moves header includes after the #define. Some @brief has been added here and there at the same time. Former-commit-id: 9beea0c

Description Everywhere throughout the include files of the core and the solver library, there were multiple includes that were defined before the #ifndef -> #define -> #endif statements. This is mostly aesthetics, but this can also have a small impact on compilation time, especially on systems with a slow filesystem (e.g. the clusters). See for example: https://stackoverflow.com/questions/20988705/is-an-include-before-ifdef-define-include-guard-okay Additionally, some of the include guards were in capital letters, some were not. Heck, some were even missing. So I took the opportunity to uniformize thing. I limited myself to core/solver as a first step, but in a follow-up PR I could do it for the other parts of the library. Testing There are not tests required here. If the code compiles, everything will be fine. Documentation No documentation was modified in the making of this PR. Co-authored-by: Audrey Collard-Daigneault <71884806+acdaigneault@users.noreply.github.com> Co-authored-by: Amishga Alphonius <107414376+AmishgaAlphonius@users.noreply.github.com> Former-commit-id: eea049f

@brief

Description Quick refactor as done in PR #1170 that moves header includes after the #define. Some @brief has been added here and there at the same time. Former-commit-id: 9beea0c

Migrate include after ifdefs in core and solver h

6ea9cb9

blaisb added the Refactoring This PR is only refactoring or clean up label Jun 12, 2024

blaisb requested review from acdaigneault and AmishgaAlphonius June 12, 2024 18:41

acdaigneault reviewed Jun 12, 2024

View reviewed changes

Update include/core/solid_objects_parameters.h

0752c5f

Co-authored-by: Audrey Collard-Daigneault <71884806+acdaigneault@users.noreply.github.com>

AmishgaAlphonius reviewed Jun 12, 2024

View reviewed changes

blaisb and others added 6 commits June 12, 2024 17:05

Update include/core/dem_properties.h

f9ddc30

Co-authored-by: Amishga Alphonius <107414376+AmishgaAlphonius@users.noreply.github.com>

Update include/solvers/postprocessors.h

46d9081

Co-authored-by: Amishga Alphonius <107414376+AmishgaAlphonius@users.noreply.github.com>

Update include/solvers/simulation_parameters.h

cd61761

Co-authored-by: Amishga Alphonius <107414376+AmishgaAlphonius@users.noreply.github.com>

Update include/solvers/postprocessors_smoothing.h

6a0b2b8

Co-authored-by: Amishga Alphonius <107414376+AmishgaAlphonius@users.noreply.github.com>

Update include/core/mesh_controller.h

32227eb

Co-authored-by: Amishga Alphonius <107414376+AmishgaAlphonius@users.noreply.github.com>

Apply all comments and indent

a608ba2

blaisb requested review from acdaigneault and AmishgaAlphonius June 12, 2024 21:42

Last comments

87ee44f

acdaigneault approved these changes Jun 12, 2024

View reviewed changes

AmishgaAlphonius approved these changes Jun 13, 2024

View reviewed changes

blaisb merged commit eea049f into master Jun 13, 2024
8 checks passed

blaisb deleted the include_in_core_headers branch June 13, 2024 12:05

acdaigneault mentioned this pull request Aug 5, 2024

Move includes after #define in DEM #1223

Merged

12 tasks

blaisb pushed a commit that referenced this pull request Aug 6, 2024

Move includes after #define in DEM (#1223)

9beea0c

Description Quick refactor as done in PR #1170 that moves header includes after the #define. Some @brief has been added here and there at the same time.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate include after ifdefs in core and solver h #1170

Migrate include after ifdefs in core and solver h #1170

blaisb commented Jun 12, 2024

acdaigneault left a comment

acdaigneault Jun 12, 2024

blaisb Jun 12, 2024

AmishgaAlphonius left a comment •

edited by blaisb

Loading

AmishgaAlphonius Jun 12, 2024

blaisb Jun 12, 2024 •

edited

Loading

acdaigneault commented Jun 12, 2024

blaisb commented Jun 12, 2024

Migrate include after ifdefs in core and solver h #1170

Migrate include after ifdefs in core and solver h #1170

Conversation

blaisb commented Jun 12, 2024

Description

Testing

Documentation

Checklist (will be removed when merged)

acdaigneault left a comment

Choose a reason for hiding this comment

acdaigneault Jun 12, 2024

Choose a reason for hiding this comment

blaisb Jun 12, 2024

Choose a reason for hiding this comment

AmishgaAlphonius left a comment • edited by blaisb Loading

Choose a reason for hiding this comment

AmishgaAlphonius Jun 12, 2024

Choose a reason for hiding this comment

blaisb Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

acdaigneault commented Jun 12, 2024

blaisb commented Jun 12, 2024

AmishgaAlphonius left a comment •

edited by blaisb

Loading

blaisb Jun 12, 2024 •

edited

Loading