-
Notifications
You must be signed in to change notification settings - Fork 878
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
v1.10 and master: mpirun without enough slots returns $status==0 #1344
Comments
I discussed the secondary question with @rhc54 in an other ticket, and this is what we want. note in master you can |
Let me just clarify a bit. I personally dislike this behavior as I find it annoying. However, the issue that was originally raised was the following: $ mpirun -host foo hostname would automatically run a number of procs equal to the number of cores on host foo, and not just a single process. This is due to our decision to equate the lack of specifying the number of procs to effectively directing that we run a number equal to the number of "slots" in the allocation, which we set to the number of cores if it wasn't specified in a hostfile. However, when you do specify the number of procs, then we still view this command line as having only one slot, which is what triggers the error. I don't believe that should be the behavior, but it is consistent with what we defined. I propose that we change this behavior. If someone doesn't specify the number of procs, then we treat it as a single slot. However, if they do specify the number of procs, then we fall back to setting the number of slots equal to the number of processing elements (cores or threads, depending on what they requested). Would that make sense? |
that sounds much better to me. can you clarify what will happen if we it will start one MPI task (so far, so good), but what happens when the MPI task invokes MPI_Comm_spawn ? some random thoughts ... with the previous behavior, it was possible to an other way to see things is |
In today's code, your example would error out due to a lack of slots, which likely isn't what the user expected. Hostfile parsing is a different subject. In that case, the behavior is more as you would expect. If you list a hostname once and don't explicitly set the number of slots, then we default to the number of discovered cores. If you list a hostname more than once, then we take that as equivalent to having included a "slots=N" statement and set the number of slots equal to the number of times you list the hostname. It is only the -host option where things differ, and I'm not convinced it is doing what people would expect. |
Just to be clear: this issue is specifically about mpirun returning a status of 0, even when there's an error. That clearly seems to be incorrect behavior. I'm guessing it should be easy to fix; I just don't know where to look in the code. The secondary question, I agree, is also quite important. @rhc54 let me see if I understand current behavior:
for i = 0 .. num_hosts
for j = 0 .. num_slots(host[i])
launch foo on host[i] Is that correct? |
I agree about this issue being more limited, but I'd rather not dive into that code multiple times if it can be avoided. So let's resolve the desired behavior. Your detailing of the current behavior is correct. |
@rhc54 Agreed that we might as well dive into this code once; I just wanted to make sure that the original issue that was reported wasn't lost in the discussion. If the description at #1344 (comment) is correct, then it seems like a no-brainer to make the Agreed? |
imho, there are currently two differences between
it seems everyone agree failing because of oversubscription is annoying and is ok to change that. i can only guess there is a rationale for starting only one MPI task with the fwiw |
I think I agree with everything @ggouaillardet is saying. To summarize:
I believe the rationale for making I do recall that there was some pushback on making |
😩 the behavior you describe is what we -did- have prior to deciding we needed to change it to the current behavior. Why don't we at least raise this at the telecon on Tues - and then let's agree to quit going back-and-forth on this topic as it is getting rather confusing to the users and frankly frustrating to me. |
@jsquyres @ggouaillardet Please see #1353 and tell me if this is okay |
Fixes opal_atomic_ll_64.
…ly run one instance if no slots were provided and the user didn't specify #procs to run. However, if no slots are given and the user does specify #procs, then let the number of slots default to the #found processing elements Ensure the returned exit status is non-zero if we fail to map If no -np is given, but either -host and/or -hostfile was given, then error out with a message telling the user that this combination is not supported. If -np is given, and -host is given with only one instance of each host, then default the #slots to the detected #pe's and enforce oversubscription rules. If -np is given, and -host is given with more than one instance of a given host, then set the #slots for that host to the number of times it was given and enforce oversubscription rules. Alternatively, the #slots can be specified via "-host foo:N". I therefore believe that row #7 on Jeff's spreadsheet is incorrect. With that one correction, this now passes all the given use-cases on that spreadsheet. Make things behave under unmanaged allocations more like their managed cousins - if the #slots is given, then no-np shall fill things up. Fixes open-mpi#1344
With the HEAD of v1.10 and master (probably v2.x, too, but I didn't check it): if
mpirun
complains about a lack of slots,mpirun
still returns an exit status of 0. Note that the test below was run on a machine with 16 cores; this test is run with the ssh launcher and not under a job scheduler (i.e., not in a SLURM job):A secondary question: did we decide that this is the behavior we wanted? The mpi001 machine has 16 cores. I remember we talked about this, but I don't remember what we decided...
Thanks to @rfaucett for identifying the problem.
The text was updated successfully, but these errors were encountered: