Skip to content

Conversation

@annu13
Copy link
Contributor

@annu13 annu13 commented Oct 3, 2015

No description provided.

@lanl-ompi
Copy link
Contributor

Test FAILed.

2 similar comments
@lanl-ompi
Copy link
Contributor

Test FAILed.

@lanl-ompi
Copy link
Contributor

Test FAILed.

@annu13 annu13 closed this Oct 3, 2015
@annu13
Copy link
Contributor Author

annu13 commented Oct 3, 2015

Bot: retest

@annu13 annu13 reopened this Oct 3, 2015
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably remove this debug, or at least replace it with pmix_output_verbose

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will replace with pmix_output_verbose.
On 10/2/15, 5:36 PM, "rhc54" notifications@github.com wrote:

In opal/mca/pmix/pmix1xx/pmix/examples/server.c
#969 (comment):

@@ -343,6 +346,20 @@ static void errhandler(pmix_status_t status,
pmix_output(0, "SERVER: ERRHANDLER CALLED WITH STATUS %d", status);
}

+static void op_callbk(pmix_status_t status,

  •                  void *cbdata)
    
    +{
  • pmix_output(0, "SERVER: OP CALLBACK CALLED WITH STATUS %d",
    status);
    +}

+static void errhandler_reg_callbk (pmix_status_t status,

  •                               int errhandler_ref,
    
  •                               void *cbdata)
    
    +{
  • pmix_output(0, "SERVER: ERRHANDLER REGISTRATION CALLBACK CALLED
    WITH STATUS %d, ref=%d",
    We should probably remove this debug, or at least replace it with
    pmix_output_verbose

    Reply to this email directly or
    view it on GitHub
    https://github.com/open-mpi/ompi/pull/969/files#r41081057.

@annu13
Copy link
Contributor Author

annu13 commented Oct 6, 2015

@rhc54, please merge this PR. We need to replace pmix_output with pmix_output_verbose in multiple places, I will clean those and submit a PR in pmix repo directly.

@jsquyres
Copy link
Member

jsquyres commented Oct 6, 2015

@rhc54 is probably going to be out for a few days.

Is there a reason you can't update this PR to be correct before we merge it?

@annu13
Copy link
Contributor Author

annu13 commented Oct 6, 2015

It is a trivial change that needs to be done in many places in this file and other files in PMIX. I thought it was easier to do it there and bring to OMPI than the other way round. Anyways I just made the changes and added it to the PR. This is good to be merged once the build is successful.

@jsquyres
Copy link
Member

jsquyres commented Oct 6, 2015

@annu13 I'm sorry; that's what I actually meant: that you would put this stuff on the PMIX master, and then add another "sync" commit over here on this PR. I agree that the direction of code should always flow from PMIX to OMPI -- not the other way around.

Have you committed this stuff on the PMIX master yet?

Also, what's the merge commit on this PR? It doesn't seem like that should be here -- i.e., it seems like that is a problem...?

@annu13
Copy link
Contributor Author

annu13 commented Oct 6, 2015

On 10/6/15, 7:30 AM, "Jeff Squyres" notifications@github.com wrote:

@annu13 https://github.com/annu13 I'm sorry; that's what I actually
meant: that you would put this stuff on the PMIX master, and then add
another "sync" commit over here on this PR. I agree that the direction of
code should
always flow from PMIX to OMPI -- not the other way around.
Have you committed this stuff on the PMIX master yet?
The debug print fix commit
(openmpi@5787e92
37db9) is not in PMIX master. We will fix it in PMIX master bring it in
when we sync next time.
Also, what's the merge commit on this PR? It doesn't seem like that
should be here -- i.e., it seems like that is a problem...?

The merge commit corresponds to syncing my fork(annu13/ompi) with
open-mpi/ompi. All the commits were picked up by git automatically. I am
not sure why that commit is causing a failure.
Currently pmix2 is broken on OMPI master, this PR bring the pmix2 fixes
from PMIX master. Could you merge the PR partially ignoring today�s
commits so we can have pmix2 working again.

Reply to this email directly or
view it on GitHub
#969 (comment).

@jsquyres
Copy link
Member

jsquyres commented Oct 6, 2015

@annu13 I'm confused -- I thought you wanted all code to flow from the PMIX tree to OMPI (and not the other way around). Can you commit the 5787e92 patch to PMIX and then re-sync over here to OMPI again? That might also get rid of the (seemingly) unnecessary merge commit...?

@annu13
Copy link
Contributor Author

annu13 commented Oct 9, 2015

fixed debug stmts in pmix master and synced with pmix master (pmix git repo rev git69c398e)

rhc54 pushed a commit that referenced this pull request Oct 9, 2015
@rhc54 rhc54 merged commit db467d1 into open-mpi:master Oct 9, 2015
jsquyres pushed a commit to jsquyres/ompi that referenced this pull request Sep 19, 2016
v1.10: Fix MPI_TYPE_SET_ATTR with NULL value
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.

4 participants