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

Removed mpich and slurm components from schizo framework #24

Closed

Conversation

Michael-uofl
Copy link

This pull request removes the mpich and slurm components from the schizo framework.

Signed-off-by: Michael Ayele <whayel01@louisville.edu>
Signed-off-by: Michael Ayele <whayel01@louisville.edu>
Signed-off-by: Michael Ayele <whayel01@louisville.edu>
@jsquyres jsquyres marked this pull request as draft October 1, 2024 19:26
@@ -42,7 +42,6 @@ series, in reverse chronological order.
- Remove event header defines
- Minor cleanups and ensure no local IOF copy
when persistent
- change the pcc wrapper compiler to a symlink
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you want to remove this: this is a historical note of what happened in a specific Open MPI v2.x release.

@@ -128,7 +127,7 @@ Testing
--prtemca prte_abort_on_non_zero_status 0 \
--debug-daemons

# using 'errmgr_detector_enable 1' choose enable the error detector.
# using 'errmgr_detector_enable 1' choose enable the error detector.
Copy link
Member

Choose a reason for hiding this comment

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

Was there a reason for which whitespace change?

@@ -145,9 +144,6 @@ Step 3: under example we have 2 test codes ``error_notify.c``,

.. code-block:: bash

# Compile the codes
pcc -g error_notify.c -o error_notify
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure the changes in this file are correct. Removing this line of text from the example documentation means that the docs no longer show the compilation step. That doesn't seem right.

(same for the other similar changes in this file)

@@ -27,6 +27,4 @@ been directed _not_ to return any collected data from calls to PMIx_Put.
dynamic.c:


The Makefile assumes that the pcc wrapper compiler is in your path.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that this statement should be removed.

@@ -24,7 +24,6 @@

# Use the PRRTE-provided wrapper compiler

CC = pcc
Copy link
Member

Choose a reason for hiding this comment

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

Do you know what removing this line does? It will definitely break the functionality of this Makefile.

This is probably worth talking about at the next meeting.

tools/prte_info \
tools/prte \
tools/pterm \
tools/psched

Copy link
Member

Choose a reason for hiding this comment

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

Nit: don't add a blank line at the end of the file.

DIST_SUBDIRS += \
tools/prted \
tools/prun \
tools/pcc \
tools/prte_info \
tools/prte \
tools/pterm \
Copy link
Member

Choose a reason for hiding this comment

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

You can't have a macro end with a continuation character; unexpected things can happen.

@@ -25,8 +25,6 @@

# Use the PRTE-provided wrapper compiler

CC = pcc
Copy link
Member

Choose a reason for hiding this comment

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

Do you know what removing this line does?

@@ -28,21 +28,14 @@
SUBDIRS += \
tools/prted \
tools/prun \
tools/pcc \
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to re-evaluate what pcc is used for, and whether it should actually be removed or not.

Copy link
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 not remove pcc, we just don't want to document it.

Copy link

Choose a reason for hiding this comment

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

Yeah, removal would be problematic in general. You could switch to "pmixcc", but then you'd need to modify all test and example compiles to point to the PMIx location and deal with those complexities (e.g., what is the user's PATH when they want to build an example).

@rhc54
Copy link

rhc54 commented Oct 1, 2024

Am I misunderstanding things? I keep seeing additional PR's popping up that are also labeled as "remove mpich and slurm components"

@Michael-uofl Michael-uofl deleted the simplify-framework branch October 7, 2024 21:37
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.

4 participants