Skip to content

Fix missing includes - #442

Draft
EmilyBourne wants to merge 4 commits into
ebourne_288_iwyufrom
ebourne_288_iwyu_fixes
Draft

Fix missing includes#442
EmilyBourne wants to merge 4 commits into
ebourne_288_iwyufrom
ebourne_288_iwyu_fixes

Conversation

@EmilyBourne

Copy link
Copy Markdown
Member

<Describe your PR here. This will form the basis of the squashed commit message in the devel branch.>


Please complete the checklist to ensure that all tasks are completed before marking your pull request as ready for review.

All Submissions

  • Have you ensured that all lines changed in this PR are justified by a comment found in the description ?
  • Have you updated the CHANGELOG.md ?
  • Have you linked any issues that should be closed when this PR is merged (using closing keywords) ?
  • Have you checked that the AUTHORS file is up to date ?
  • Have you checked that the copyright information in the LICENCE file is up to date (including dates) ?
  • Do you follow the conventions specified in our coding standards ?

New Feature Submissions

  • Have you added tests for the new functionalities ?
  • Have you documented the new functionalities:
    • API documentation describing the available methods, when each should be used and how to use them ?
    • User-friendly documentation in README files (which may link to the API documentation).
    • If the new functionality is non-trivial to use, provide a tutorial or example ? (optional)

Changes to Existing Features

  • Have you checked that existing tests cover all code after the changes ?
  • Have you checked that existing tests are still passing ?
  • Have you checked that the existing documentation is still accurate (API and README files) ?

Changes to the CI

  • Have you made the same changes to both the GitHub CI and the GitLab CI (for the private fork) ?

@EmilyBourne

Copy link
Copy Markdown
Member Author

@tpadioleau This PR contains the results of the following command:

bin/run_iwyu toolchains/common_toolchains/serial_debug_toolchain.cmake src/ tests/ simulations/ -j 4 --fix; ./bin/indent -d

There will probably be some wrong suggestions but I have pushed as is to help evaluate #435

Comment thread src/collisions/collision_operator.cpp
Comment thread simulations/geometryXY/guiding_centre/guiding_centre.cpp
@EmilyBourne
EmilyBourne force-pushed the ebourne_288_iwyu_fixes branch from 6dcfebe to 08d5fae Compare October 9, 2025 10:28
@EmilyBourne

Copy link
Copy Markdown
Member Author

Updated following updates to run_iwyu

Comment thread src/io/input.hpp

#include "ddc_aliases.hpp"
#include "mesh_builder.hpp"
#include "non_uniform_interpolation_points.hpp"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This removal seems to create compilation problems related to is_non_uniform_interpolation_points_v

#include <ddc/ddc.hpp>
#include <ddc/kernels/splines.hpp>

#include <Serial/Kokkos_Serial.hpp>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be a private Kokkos header. Does it compile fine ? Some Kokkos headers are protected.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, I only set the impl folder in Kokkos to private

#include <Kokkos_Core.hpp>
#include <Kokkos_Macros.hpp>
#include <Kokkos_MinMax.hpp>
#include <Kokkos_Parallel_Reduce.hpp>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also private


#include <gtest/gtest.h>

#include <Kokkos_ExecPolicy.hpp>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also private

Comment thread src/geometryXVx/poisson/qnsolver.hpp Outdated

#pragma once
#include "ddc_aliases.hpp"
#include <mdspan/mdspan.hpp>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't know what is the best way to handle mdspan. Should we consider that Kokkos_Core.hpp brings it ? As Kokkos does for atomics that come from the desul library ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤔 I think we might occasionally use mdspan, but I'm not sure why it thinks it's needed here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Interesting:

#include <mdspan/mdspan.hpp> // for layout_right

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same issue in DDC.

#include <ddc/pdi.hpp>

#include <Kokkos_Core.hpp>
#include <Kokkos_Layout.hpp>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure if it is private

#include <ddc/ddc.hpp>
#include <ddc/pdi.hpp>

#include <Kokkos_Concepts.hpp>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure either if it is private

#include <ddc/ddc.hpp>
#include <ddc/kernels/splines.hpp>

#include <Serial/Kokkos_Serial.hpp>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Private

#include <gtest/gtest.h>

#include <Kokkos_Core.hpp>
#include <Kokkos_ExecPolicy.hpp>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Private

#include <Kokkos_ExecPolicy.hpp>
#include <Kokkos_Macros.hpp>
#include <Kokkos_MinMax.hpp>
#include <Kokkos_Parallel_Reduce.hpp>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Private

Comment thread tests/matrix_tools/matrix_batch_ell.cpp Outdated
#include <Kokkos_ExecPolicy.hpp>
#include <Kokkos_Layout.hpp>
#include <Kokkos_Macros.hpp>
#include <Kokkos_Parallel.hpp>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Private


#include <Kokkos_Core.hpp>
#include <Kokkos_Macros.hpp>
#include <stdlib.h>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Weird, why not ? Is there a missing qualification std:: ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah probably:

#include <stdlib.h>                                  // for abs

this may be why I had that abs line

#include <Kokkos_Core.hpp>
#include <mdspan/mdspan.hpp>

#include <string.h>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should use cstring with std::memset

#include <ddc/ddc.hpp>

#include <gmock/gmock.h>
#include <ext/type_traits.h>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Who are you ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This doesn't help much:

#include <ext/type_traits.h>               // for __promote_2

🤔

/home/emily/Code/gyselalibxx/tests/geometryXVx/lagrange_test.cpp:136:61: warning: __gnu_cxx::__promote_2 is defined in <ext/type_traits.h>, which isn't directly #included.

LagrangeTest<100> Test_instance_hundred(100, deg, true, [](double x, int d) {
return std::pow(x, d);
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Then just a limitation of iwyu


#include <gtest/gtest.h>

#include <math.h>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Missing std:: qualification ?

@EmilyBourne
EmilyBourne force-pushed the ebourne_288_iwyu_fixes branch from 08d5fae to fc6258f Compare October 14, 2025 09:10
@EmilyBourne
EmilyBourne force-pushed the ebourne_288_iwyu_fixes branch from fc6258f to 085f72f Compare October 14, 2025 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants