Skip to content

Added HPX::auto_wrap_main option #6804

Merged
hkaiser merged 9 commits intoTheHPXProject:masterfrom
kollanur:dev-link
Nov 27, 2025
Merged

Added HPX::auto_wrap_main option #6804
hkaiser merged 9 commits intoTheHPXProject:masterfrom
kollanur:dev-link

Conversation

@kollanur
Copy link
Copy Markdown
Contributor

@kollanur kollanur commented Nov 15, 2025

Added the HPX::auto_wrap_main CMake target that excludes the need of including <hpx/hpx_main.hpp> header file in the main function to start HPX runtime when HPX::wrap_main is linked.

@kollanur kollanur requested a review from hkaiser as a code owner November 15, 2025 20:27
@hkaiser hkaiser added this to the 2.0.0 milestone Nov 15, 2025
@hkaiser hkaiser changed the title Added -HPX_WITH_WRAP_MAIN_AUTO_ACTIVATE CMake option to automatically link HPX::wrap_main without the need of header file <hpx/hpx_main.hpp> in the user main() function. Added HPX_WITH_WRAP_MAIN_AUTO_ACTIVATE CMake option Nov 15, 2025
@StellarBot
Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

Copy link
Copy Markdown
Contributor

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

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

Could you add a test verifying this functionality, please? Perhaps here: https://github.com/STEllAR-GROUP/hpx/tree/master/tests/unit/init?

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Nov 16, 2025

@kollanur
Copy link
Copy Markdown
Contributor Author

Could you add a test verifying this functionality, please? Perhaps here: https://github.com/STEllAR-GROUP/hpx/tree/master/tests/unit/init?

Yes, I will do that.

@kollanur
Copy link
Copy Markdown
Contributor Author

Some of the CIs are also failing because of an unused argument (https://app.circleci.com/pipelines/github/STEllAR-GROUP/hpx/18976/workflows/1db954bb-64fe-4130-81a8-15edc2925fbd/jobs/435313).

I will check those failures and fix them.

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Nov 16, 2025

After thinking about this a bit more - we either should make this the default and make sure that the existing code still works (the one that has the #include <hpx/hpx_main.hpp>), or alternatively make it a buy-in on a application-per-application basis, preferrably by creating a dependency to a special target.

@kollanur
Copy link
Copy Markdown
Contributor Author

Some of the CIs are also failing because of an unused argument (https://app.circleci.com/pipelines/github/STEllAR-GROUP/hpx/18976/workflows/1db954bb-64fe-4130-81a8-15edc2925fbd/jobs/435313).

I will check those failures and fix them.

Fixed it.

@kollanur
Copy link
Copy Markdown
Contributor Author

After thinking about this a bit more - we either should make this the default and make sure that the existing code still works (the one that has the #include <hpx/hpx_main.hpp>), or alternatively make it a buy-in on a application-per-application basis, preferrably by creating a dependency to a special target.

I have made it default with the exisiting code still working.

…he need of including <hpx/hpx_main.hpp> header file in the main function to start HPX runtime when HPX::wrap_main is linked

Signed-off-by: Bharath <bharath.kollanur@gmail.com>
…used argument error

Signed-off-by: Bharath <bharath.kollanur@gmail.com>
@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Nov 18, 2025

After thinking about this a bit more - we either should make this the default and make sure that the existing code still works (the one that has the #include <hpx/hpx_main.hpp>), or alternatively make it a buy-in on a application-per-application basis, preferrably by creating a dependency to a special target.

I have made it default with the exisiting code still working.

It looks like that many tests are now failing. So making it the default wasn't a good idea after all :/

kollanur and others added 2 commits November 22, 2025 14:49
@kollanur kollanur changed the title Added HPX_WITH_WRAP_MAIN_AUTO_ACTIVATE CMake option Added HPX::auto_wrap_main option Nov 22, 2025
Comment thread tests/unit/init/auto_wrap_main.cpp
Comment thread tests/unit/init/CMakeLists.txt
Comment thread wrap/src/hpx_auto_wrap.cpp Outdated
Comment thread wrap/CMakeLists.txt Outdated
kollanur and others added 3 commits November 24, 2025 11:47
Signed-off-by: Bharath <bharath.kollanur@gmail.com>
…nctionality in the hpx_wrap.cpp which is defined to the target the with the help of target_compile_definitions in the CMakelists.txt file

Signed-off-by: Bharath <bharath.kollanur@gmail.com>
Comment thread wrap/src/hpx_wrap.cpp
Comment thread wrap/src/hpx_wrap.cpp Outdated
Comment thread wrap/src/hpx_wrap.cpp Outdated
hkaiser
hkaiser previously approved these changes Nov 26, 2025
Copy link
Copy Markdown
Contributor

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

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

Very nice, LGTM, thanks!

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Nov 26, 2025

@kollanur please disregard my last comment about using <hpx/modules/init_runtime_local.hpp>. I was focussing on the HPX_APPLICATION_STRING macro and missed that you needed <hpx/init.hpp> because of the calls to hpx::init. It is not sufficient to initialize HPX using hpx::local::init as this will not bring up the distributed runtime environment. I think you can safely roll back your latest commit. We can merge this PR once you do.

@kollanur
Copy link
Copy Markdown
Contributor Author

@kollanur please disregard my last comment about using <hpx/modules/init_runtime_local.hpp>. I was focussing on the HPX_APPLICATION_STRING macro and missed that you needed <hpx/init.hpp> because of the calls to hpx::init. It is not sufficient to initialize HPX using hpx::local::init as this will not bring up the distributed runtime environment. I think you can safely roll back your latest commit. We can merge this PR once you do.

ok professor, got it.

…N_STRING

Signed-off-by: Bharath <bharath.kollanur@gmail.com>
@hkaiser hkaiser merged commit 4dacbac into TheHPXProject:master Nov 27, 2025
71 of 72 checks passed
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.

3 participants