-
Notifications
You must be signed in to change notification settings - Fork 2
Removed mpich and slurm components from schizo framework #24
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
Conversation
Signed-off-by: Michael Ayele <whayel01@louisville.edu>
Signed-off-by: Michael Ayele <whayel01@louisville.edu>
Signed-off-by: Michael Ayele <whayel01@louisville.edu>
@@ -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 |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 | ||
|
There was a problem hiding this comment.
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 \ |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 \ |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
Am I misunderstanding things? I keep seeing additional PR's popping up that are also labeled as "remove mpich and slurm components" |
This pull request removes the
mpich
andslurm
components from theschizo
framework.