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

datatype: Fix pack external by using the MPIR layer #7336

Merged
merged 1 commit into from
Mar 14, 2025

Conversation

dalcinl
Copy link
Contributor

@dalcinl dalcinl commented Mar 13, 2025

Pull Request Description

Fixes #7313.

This PR could be an opportunity to refactor/consolidate the implementation and use of the following two routines.

// src/mpi/datatype/typerep/src/typerep_util.h
bool MPII_Typerep_basic_type_is_complex(MPI_Datatype el_type);
bool MPII_Typerep_basic_type_is_unsigned(MPI_Datatype el_type);

@hzhou Feel free to push/force-push to my branch, or pick the commit and continue in your clone.

Author Checklist

  • Provide Description
    Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
  • Commits Follow Good Practice
    Commits are self-contained and do not do two things at once.
    Commit message is of the form: module: short description
    Commit message explains what's in the commit.
  • Passes All Tests
    Whitespace checker. Warnings test. Additional tests via comments.
  • Contribution Agreement
    For non-Argonne authors, check contribution agreement.
    If necessary, request an explicit comment from your companies PR approval manager.

@dalcinl
Copy link
Contributor Author

dalcinl commented Mar 13, 2025

As the regression this PR is fixing was not catched by MPICH tests, I guess mpi4py is the best we have.
Test are passing in my workstation with ch3:nemesis, but let's run a more comprehensive CI matrix:
https://github.com/mpi4py/mpi4py-testing/actions/runs/13841348758

Copy link
Contributor

@hzhou hzhou left a comment

Choose a reason for hiding this comment

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

LGTM. I'll run CI tests and merge after it clears.

@dalcinl Sorry for neglecting your issue. Thanks for the fix!

@hzhou
Copy link
Contributor

hzhou commented Mar 13, 2025

This PR could be an opportunity to refactor/consolidate the implementation and use of the following two routines.

Good point! I'll review and make a new PR if we decide to do the clean up.

@hzhou
Copy link
Contributor

hzhou commented Mar 13, 2025

test:mpich/warnings
test:mpich/ch3/most

@dalcinl
Copy link
Contributor Author

dalcinl commented Mar 14, 2025

@hzhou Looks like something is off in your Jenkins server.

@hzhou
Copy link
Contributor

hzhou commented Mar 14, 2025

test:mpich/warnings

@hzhou
Copy link
Contributor

hzhou commented Mar 14, 2025

@hzhou Looks like something is off in your Jenkins server.

They occasionally have hiccups 😄

@hzhou hzhou merged commit 4912a1a into pmodels:main Mar 14, 2025
2 of 4 checks passed
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.

mpi4py: Regressions after recent datatype refactoring
2 participants