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

yaksa: move yaksa into mpich #7284

Merged
merged 5 commits into from
Feb 6, 2025
Merged

yaksa: move yaksa into mpich #7284

merged 5 commits into from
Feb 6, 2025

Conversation

hzhou
Copy link
Contributor

@hzhou hzhou commented Jan 30, 2025

Pull Request Description

Move yaksa into mpich repo, potentially to allow less redundancy and
less interface overhead.

This is a clone from yaksa commit 9e307612e333a133357030d1b2f3dde4d9d43cce.

The external yaksa will be kept as is so any application that uses yaksa
will continue work. The internal yaksa will diverge this point on.

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.

Sorry, something went wrong.

@hzhou hzhou force-pushed the 2501_yaksa branch 2 times, most recently from a108152 to 3d321a1 Compare January 30, 2025 16:28
@hzhou
Copy link
Contributor Author

hzhou commented Jan 30, 2025

test:mpich/ch3/most
test:mpich/ch4/most

@hzhou hzhou requested a review from raffenet January 30, 2025 16:44
@hzhou
Copy link
Contributor Author

hzhou commented Feb 3, 2025

test:mpich/ch4/ofi

Copy link
Contributor

@raffenet raffenet left a comment

Choose a reason for hiding this comment

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

This look fine to me. I do wonder if there is anything useful in the test directory that's not otherwise covered by the MPICH tests, but I guess we can always resurrect that code via the git history if desired.

@@ -240,9 +240,9 @@ AC_CHECK_ALIGNOF(long double)

# backend devices
supported_backends="seq"
m4_include([src/backend/cuda/subconfigure.m4])
Copy link
Contributor

Choose a reason for hiding this comment

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

This feature in gen_subcfg_m4 should also work to avoid this issue if a place a sentinel in the top yaksa directory?

# the existence of this file means we should stop recursing down
# the enclosing directory tree
my $stop_sentinel = ".no_subcfg_recursion";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yaksa manually include those subconfigure files anyway, so I think rename them is cleaner. I am afraid the stop_sentinel mechanism is too subtle and may confuse people sometime.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's subtle either way that we have to avoid these filenames in the repo. But any more time spent on this aspect is not useful, so let's just ship it.

hzhou added 5 commits February 6, 2025 14:43
Move yaksa into mpich repo, potentially to allow less redundancy and
less interface overhead.

This is a clone from yaksa commit 9e307612e333a133357030d1b2f3dde4d9d43cce.

The external yaksa will be kept as is so any application that uses yaksa
will continue work.  The internal yaksa will diverge this point on.
We need prevent yaksa subconfigure files to be scraped by mpich's
gen_subcfg.m4.
The yaksa tests are mostly redundant to mpich's dtpool tests. They
assumes yaksa's interface independent of MPI. That will not be
maintained after we moved yaksa inside mpich. Thus, we are removing
these tests.
@hzhou hzhou merged commit 28e9057 into pmodels:main Feb 6, 2025
4 checks passed
@hzhou hzhou deleted the 2501_yaksa branch February 6, 2025 22:07
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.

None yet

2 participants