-
Notifications
You must be signed in to change notification settings - Fork 395
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
multiprocess functionality does not play way with generators #125
Comments
After playing around with the multiprocess.py file a bit, I got a hacked up version to add tests from generates back to the queue. This was a bit tricky because function args had to be also passed. I'm attaching the new multiprocess.py file and its diff, hopefully you guys have a better way of implementing this. Google Code Info: |
Google Code Info: |
i forgot to mention that the biggest change that allowed this to happen is overriding the ContextSuite.run method with an implementation that queues up self._tests to testQueue if len(self._tests) > 1. rosen diankov, Google Code Info: |
this fixes several bugs when passing the results back to the main process. rosen dainkov, Google Code Info: |
this fixes bugs when the generator function throws an exception rosen diankov, Google Code Info: |
It turns out that the timeout feature of the multiprocess plugin was also broken. My expectation was that a timeout of 1s would immediately stop all tests that exceeded 1s value. But, the current version of multiprocess just put time outs on the queues, and just printed "timed out" messages, which is not so useful. I'm attaching yet more patches, that sends SIGINT to each of the workers and when a timeout occurs, they gracefully stop their current test and return a failure. Google Code Info: |
one more small change: the time out failures now return a multiprocess.TimedOutException exception to be easily identifiable rosen diankov, Google Code Info: |
Instead of posting new versions, you can find the latest multiprocess.py with these fixes here: https://openrave.svn.sourceforge.net/svnroot/openrave/trunk/test/noseplugins/multiprocess.py rosen diankov, Google Code Info: |
Google Code Info: |
Hello. Thanks for following up with the patches. One minor comment: please avoid decorators like @staticmethod so that the code will compile without syntax errors in older pythons (even though this plugin won't be used). You can just double it up like address = staticmethod(address). Currently with this patch I get this failure in tox -e py26 ERROR: test_multiprocess.test_mp_process_args_pickleableTraceback (most recent call last): Also, can you add a test case using the generator example above? If you get stuck, I can help with that. To run the test suite, install tox and type tox -e py26 at the root of the nose source. http://codespeak.net/tox/ Google Code Info: |
this seems like a simple problem to solve, i'll patch it in a day or two and test with tox. about staticmethod, i'll just make them regular methods that don't use self about decorator test case...... i looked at the test cases for the current multiprocess plugin and it will take me some time to figure things out and write a correct test case. perhaps you could help? Google Code Info: |
hi kumar, it doesn't look like you were using the latest code from: https://openrave.svn.sourceforge.net/svnroot/openrave/trunk/test/noseplugins/multiprocess.py Google Code Info: |
Hi Rosen. I tried out this link but I got the same pickle test failure as above. That's the main thing to fix since I'm not entirely sure why that fails. As for creating a regression test, I can try to make one so consider that low priority if you get stuck on it. Google Code Info: |
adding empty comment to trick groups into resending previous messages which I accidentally moderated into the trash. Google Code Info: |
ok, this took me a while to figure out, but the problem was not in the new multiprocess.py plugin, it was in the test_multiprocess.py test. The original one was ignoring the first 4 parameters when pickling (these are the sync primitives). The new one has increased sync primitives, so it has to ignore the first 7 parameters when pickling. It was also not popping stuff off the testQueue, causing an infinite loop. I'm attaching the new test_multiprocess.py You should get the latest multiprocess.py from the url above (doc updates). Google Code Info: |
Hi Rosen, thanks for digging into the tests. This patch is almost there. The python 2.6 tests are fixed but when I patched with the latest code from your svn link above I got a failure in python 3 that I don't understand. Can you take a look? I cleaned up your patch for PEP8 and other minor things so please submit a diff against my fork here: Are you able to run tox -e py32? (also same failure with py31)
I don't know why Array('c',' '*1000) works in 2.6 but not 3. Can you think of a workaround? Google Code Info: |
that's weird, i just tried it on python 3.1 without problems: {{{ And Array does work: {{{
Unfortunately my linux distro does not have a python 3.2 package so i can't test it... Google Code Info: |
oh, it does look like this problem is limited to Python 3.2 actually (my bad). On Linux, you can just download the Python 3 source and type ./configure && make && make install -- it will not clobber your existing python and you'll get a binary as /usr/local/bin/python3.2 You can pass a flag to configure to install it in a custom place for temporary use. For it to work in tox all you have to do is make sure python3.2 is on $PATH Google Code Info: |
Sorry for the late response, I think i found a common ground. here are the patches Google Code Info: |
Hi Rosen, this is still failing and actually I don't see any differences in your patch. I had to apply it by hand though because you didn't diff against the branch I'm working in: https://bitbucket.org/kumar303/nose-multi (maybe I missed something?) Can you clone that branch above, run tox -e py32, and take a look? I get the same thing: ERROR: test_multiprocess.test_mp_process_args_pickleableTraceback (most recent call last): Google Code Info: |
i just noticed that the patch is in reverse! ;0) the correct code is: Google Code Info: |
You can apply reverse patches by passing the '-R' option to patch Google Code Info: |
After some tweaks to make multiprocessing use byte values in all versions of Python, the tests are finally passing. Can you try installing Nose from the branch to see if it works in some real world multiprocessing suites? pip install -e hg+https://bitbucket.org/kumar303/nose-multi#egg=nose Google Code Info: |
If you made the exact changes i gave you, then it shouldn't be a problem. i've already tested them. Using the command you gave gives an error: {{{ Storing complete log in ./pip-log.txt Google Code Info: |
What version of pip do you have? Mine is 0.8.1 and I do not get that error using python 2.6. Judging from pip.py it looks like you might have a really old version. Google Code Info: |
you were right, it as too old. just updated to pip 1.0 and tested with no problems. awesome work! Google Code Info: |
Excellent, thanks for testing. This has been merged into nose's main repo and will go out in 1.0.1 Google Code Info: |
Hmm, it seems that test_mp_process_args_pickleable() gets caught in an infinite loop but not all of the time. Can you take a look? try running it in a while loop so that you can catch when it gets stuck: while true; do tox -e py32; done After KeyboardInterrupt I see the following traceback. I added some debugging and it appears that the task is timing out without sending any results. I upped the timeout but it doesn't fix it. Note that I am skipping this test for now so that our CI server doesn't explode. Traceback (most recent call last): Google Code Info: |
The patch is in fact much smaller than it looks. 95% of the change set is simply de-denting a large section of code. A whitespace insensitive diff will show the important differences. I'll take a quick look at adding a test right now, but if it takes more than 30 minutes, it may need to wait till the weekend, or have someone else do it. Can you give any hints as to how specifically to add the testing? While running the tests, I'm seeing bunches of issues I could very easily help out with. Google Code Info: |
If you get stuck you could also send me a small test suite that will reproduce the problem when run with multiprocess and I can convert it into a regression test. As for contributing, the best way is to fork the repo on bitbucket https://bitbucket.org/kumar303/nose and commit small changes (in case we need to cherry pick). Thanks! Google Code Info: |
I'm nearly done, but I'm having trouble capturing the bad KeyboardInterrupt behavior. When run at the commandline, I see this:
Ran 1 test in 1.255s FAILED (errors=1) But my doctest is not seeing the "Process-1" section via nose.plugins.plugintest.run_buffered(), so it is improperly passing. Google Code Info: |
This is what I have so far. The one-line edit to multitprocess.py is needed to expose the improperly passing keyboarderror test. Google Code Info: |
I've created a patch-queue here that I'm happy with. https://bitbucket.org/bukzor/multiprocessing/qseries Do you prefer that I present this as a "real" clone? Google Code Info: |
The only cleanup that you may want to do is in the functional_tests/test_multiprocessing/mp_helper.py That file is 99% the same as nose.plugins.plugintest, but I wasn't sure how you would merge the two. You might want to use the MPFile whenever you see that the multiprocess plugin is active? Google Code Info: |
I've been doing some more testing and cleanup, but I've gotten stuck. How do I make my new multiprocessing tests be skipped when multiprocessing is not available? I've tried to copy the method in functional_tests/doctest/test_multiprocess, but I wasn't successful. I don't understand how the _fixtures.py is hooked into the doctest. Google Code Info: |
RTFM: I got it. I should have a fully test patch set for all of these issues shortly. Google Code Info: |
The patchset now passes tox on all versions of python but python3. I'm getting this strange failure. Any clues? FAIL: Doctest: test_keyboardinterrupt.rstTraceback (most recent call last): File "/home/bgolemon/trees/multiprocessing/build/tests/functional_tests/test_multiprocessing/test_keyboardinterrupt.rst", line 5, in test_keyboardinterrupt.rst
Ran 337 tests in 7.715s FAILED (SKIP=10, failures=1) Google Code Info: |
Looks like bytes_() is not getting a str/unicode object. Maybe w.currentaddr.value is a None type or something? This might fix it: bytes_(w.currentaddr.value or '', 'ascii') Google Code Info: |
that doesn't make sense, you can see that currentaddr always gets set to currentaddr.value = bytes_('') Can you try printing currentaddr.value using log.info? Google Code Info: |
That's exactly the problem. bytes() won't take bytes as an argument.
Google Code Info: |
I misspoke. The problem is in bytes_() not bytes(). See: (Pdb) bytes_(b'') Google Code Info: |
hmm...... looking from the definition of bytes_, it should be calling bytes on python 3.x {{{ Google Code Info: |
Shouldn't you really be using a string here, rather than bytes? Google Code Info: |
ffx: bytes with no decoding works fine, bytes with an explicit encoding doesn't accept bytes (since it can't be decoded). The same is true for int: (Pdb) int(10) There's a discussion here: Google Code Info: |
it was originally all strings, but due to a python 3.x quirk, the following does now work: currentaddr = Array('c',1000) if you look at the beginnings of the thread, you can read about this issue. the original solution was to do b'', but that got modified by kumar to bytes_. Google Code Info: |
The only option I see is to alter bytes_() to return early if the input is bytes. Kumar: does that seem ok, assuming it will pass tests? Google Code Info: |
i guess you can always have the default encoding as None? would this work? Google Code Info: |
No it's required to be a string, although that doesn't make sense to me personally. Google Code Info: |
Blerg! https://bitbucket.org/bukzor/multiprocessing writing manifest file '/home/bgolemon/trees/multiprocessing/build/lib.linux-x86_64-3.1/nose.egg-info/SOURCES.txt' Google Code Info: |
Hi Buck, thanks for all your work so far! I think this is the right thing to do: {{{ Also, the tests are passing in tox after the selftest.py change, which you can see in our CI machine: so I'm not sure how you got that error. I'll have some time in the next couple days to review and apply your patch series. Google Code Info: |
Great! Thanks. If you check that change in, I won't have to add it to my patch series. There is a difficulty with my MultiProcessFile. While it does correctly capture all the output, even during multiprocessing, there are no guarantees on ordering, which is problematic for doctests. To get a deterministic ordering I'd need to cache all the processes' output separately, then do an ordered concatenation during read(). Does that sound like a good idea? The memory implications of caching all output worries me, but I guess that's no different from the original StringIO implementation. This interesting enough that I'll probably spend a few hours on it tonight after work. Let me know your opinion so I can steer development in the right direction. Google Code Info: |
hi guys, excellent work solving all the problems, it's impressive. because the current xunit plugin will not work with multiprocess, i have attached some patches to it: perhaps someone will be interested in checking it out... Google Code Info: |
https://bitbucket.org/bukzor/multiprocessing This patchset is passing tox, except for what seems to be a newly-revealed bug. Google Code Info: |
I've finally completed this work. You can find it here: These changes pass tox on all platforms, and I've taken care to make each of the changes fairly small and comprehensible. Please considering merging. I've attached the tox log, since i've spent a full week to get it looking this way :) Google Code Info: |
Rosen and Buck, thanks for all your work on this! I've merged the final patches into the stable branch. Google Code Info: |
Great! I'm going to delete that fork now. Google Code Info: |
Hello, It looks like I'm getting something like this problem now. When running |
I am also seeing my generated tests run sequentially, under Python 2.7 + nose 1.3.7. |
What steps will reproduce the problem?
1.create a simple test generator function like:
def test_foo():
for i in range(10):
yield foosleep, i
def foosleep(n):
time.sleep(4)
What is the expected output? What do you see instead?
Using --processes=10, all tests should complete in ~4s.
Instead, it takes 40s since the next test does not start until the last is finished.
What version of the product are you using? On what operating system?
1.0.0 on ubuntu 10.04 x86-64
Please provide any additional information below.
i'm guessing we need a way to collect all tests before they are sent to the parallelizer?
Google Code Info:
Issue #: 399
Author: [email protected]
Created On: 2011-02-19T09:04:15.000Z
Closed On: 2011-05-02T19:59:00.000Z
The text was updated successfully, but these errors were encountered: