Skip to content

ops: add support for user-defined big count ops #13030

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

Merged

Conversation

hppritcha
Copy link
Member

related to #12226 and #9194

remove incorrect and misleading comment about ompi_3buff_op_reduce. See #967

*/
if( OPAL_UNLIKELY(full_count > INT_MAX) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The cast to int I mentioned on the previous ticket is still problematic (https://github.com/open-mpi/ompi/pull/13030/files#diff-dfd595860dc4d0c9c73114bd8563c0786d063327f6e30955a3e361c3f6098e3dR519). I can file a PR to fix if you prefer handling that separately.

Copy link
Member Author

Choose a reason for hiding this comment

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

please do.

Copy link
Contributor

Choose a reason for hiding this comment

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

Will do once this one is in.

Copy link
Member Author

Choose a reason for hiding this comment

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

okay moved that assert ahead of the cast.

related to open-mpi#12226 and open-mpi#9194

remove incorrect and misleading comment about ompi_3buff_op_reduce.
See open-mpi#967

Signed-off-by: Howard Pritchard <[email protected]>
@hppritcha hppritcha force-pushed the add_support_for_user_defined_bigops branch from 5e653ab to c484f68 Compare January 10, 2025 15:09
Copy link
Contributor

@devreal devreal left a comment

Choose a reason for hiding this comment

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

Should this PR also add MPI_Op_create_c or is that separate?

@hppritcha
Copy link
Member Author

nope, that's part of mega PR #12226

Copy link
Contributor

@devreal devreal left a comment

Choose a reason for hiding this comment

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

Changes look good to me but @bosilca should review as well.

@hppritcha
Copy link
Member Author

@bosilca please check when you have a chance

@hppritcha hppritcha merged commit c71d630 into open-mpi:main Jan 18, 2025
15 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.

2 participants