pymad / cpymad

cPyMAD is a python interface to Mad-X using cython and libmadx
http://cern.ch/pymad
Apache License 2.0
3 stars 3 forks source link

Fix setup26 #31

Closed coldfix closed 10 years ago

coldfix commented 10 years ago

I'm sorry, I missed again that include_package_data is only in setuptools. For some reason the setuptools+cython problem magically disappeared in my python26 installation. Maybe something was fixed in setuptools/python/gcc/cython or somewhere else. Could you check if the problem persists in your setup? Is it possible to check branches other than master on the CDash board?

If using setuptools is still not possible, I can restore the old package_data code.

coldfix commented 10 years ago

I have now set up a preliminary config script for the Travis CI. Compared to CDash the advantage of Travis is, that it can automatically run the tests against a matrix of different software versions (python, possibly MAD-X). Furthermore, it executes the test each time any commits are pushed to github - even for feature branches, such that it can be reviewed if anything is broken before merging to master.

If you think this would be a good idea, the usage of Travis is simple: from your Travis account, just turn the repository of your choice "on" and add one .travis.yml config file to its root directory.

As you can see, the setuptools+cython seems to work here on python26 as well. I wonder if the problem still persists on the CDash test setup.

The test script is not yet very optimal since it rebuilds MAD-X and numpy each time before performing the tests which takes about 5 to 10 minutes. I think it would be better to use a binary distribution. What do you use for the CDash solution, currently?

Also, I have not yet added all the tests. If there is any simple cmake command to execute all tests, it could be inserted in the script: section in the config file.

Eothred commented 10 years ago

Yes, we can check any branch. Perhaps we could make a branch "testing" where new stuff go before master? Alternatively we make a branch "stable" which we push to from master after testing has been done.

The SLC is generally quite conservative, so it could be that some things were fixed in python 2.6 which is not yet updated on SLC6. I will check.

Thanks for the suggestion of using Travis, and the file. I will give it a try, if nothing else than for the purpose of learning an alternative tool.

Eothred commented 10 years ago

I merged the travis file into master. From the looks of it I think it seems interesting. The only problem I have with it is that it looks like we are only testing on one platform? I think for a code which is used at CERN it is essential to test it on an SLC machine (because it is often a rather different environment).

Using Travis for continuous integration, and CDash for nightly testing on various platforms might be a good solution. If this yml-file is the only thing needed then it is minimal amount of extra work. At least for the time being.

coldfix commented 10 years ago

Yes, unfortunately Travis is restricted to a single platform, so this can't replace the CDash tests. But still I think it might be useful to see whether the branch fails on any python version.

In addition to the .yml file you will need to create an account on Travis (maybe you can use your github account, I don't remember) as well as the first commit in this pull-request and an equivalent fix for the second commit to have the test succeed.

I guess one more problem with Travis is that the tests are restricted to something like 50 minutes. This will not be enough to run all the LHC configurations etc.

Eothred commented 10 years ago

Yes, I could use the github account, and yes, was very straight forward (already merged it into master). For the time being I think we keep both, I think Travis looks more suitable for continuious integration.

I do notice that we are recompiling Mad-X for every test (three times), so I am already impressed they give us this much computing time for free :)

coldfix commented 10 years ago

Great. I think a 'testing' branch would be very useful to test some stuff without merging every mistake to master. In general I am not afraid to force-push non-mainline branches when I make a mistake and notice it in time.

Eothred commented 10 years ago

I am afraid I have to reject this fix for the time being. On our testing SLC6 machine, madx.c is never created (and as a consequence, madx.so). I don't mind the change to MANIFEST.in, so I will add that to master.

Eothred commented 10 years ago

I pushed my proposed change to the new testing branch. I don't think there are any cleaner ways to do this for the moment (though I fully agree that your solution was way simpler had it worked on SLC6).

The "deprecated" code is all inside an if statement, so can easily be removed as soon as we can. It is only used for python<2.7. Let me know if you have anything further to add.

coldfix commented 10 years ago

Ok, but can you keep the branch open for now? So I can later on use it to check for possible solutions of the python26+cython problem.

coldfix commented 10 years ago

I have investigated a bit and found a possible fix for the problem, which I want to try in the testing branch. I have created a named branch (fix-distutils-on-py26) to hold the previous content of testing (succeeded).

coldfix commented 10 years ago

The CDash tests seems to indicate that the fix works. According to my understanding another fix for the problem would be to run pip install setuptools --upgrade on the affected systems. The two other commits (e21cb12, 4bd9db9) should be merged either way, as it allows to do pip install cern-pymad on systems without Cython (I will upload the PyPI package as soon as pending pull-requests are processed).

Sorry for all the noise. I can be somewhat obnoxious when dealing with code that doesn't generalize well. I hope it's not too troublesome.

Eothred commented 10 years ago

Hi,

Yes, I guess pip could be used to upgrade the SLC6 environment, but I would prefer not to require this unless absolutely needed. There might be other python packages for SLC6 (specially developed at CERN) which have tailored setup for the exact versions.

Sorry for closing, I was a bit quick on the trigger there (want to get home for the weekend and all).

I will have a look. If it works well on SLC6 as well then I am pleased (and grateful, thanks!).

Eothred commented 10 years ago

Very nicely done!

The only thing I notice is this (but perhaps it is unrelated?):

$ python setup.py build ('missing cimport', 'src/cern/madx.pyx') cern.libmadx.madx_structures

('missing cimport', 'src/cern/libmadx/table.pyx') cern.libmadx.madx_structures libc.stdlib numpy cpython /usr/lib64/python2.6/site-packages/Cython/Includes/libc/stdlib.pxd /usr/lib64/python2.6/site-packages/Cython/Includes/numpy.pxd /usr/lib64/python2.6/site-packages/Cython/Includes/cpython/init.pxd ...

coldfix commented 10 years ago

This isn't directly related to the bug fix. It is a warning issued when using cythonize instead of Cython.Distutils.build_ext (e21cb12). I can later try to find out if this warning indicates a problem with the code or if it can be safely ignored.

coldfix commented 10 years ago

I don't quite understand why the warning is issued, when it doesn't find a '.pxd' file for a '.pyx' file. I don't think it indicates any real problem. It can be disabled by invoking cythonize(..., quiet=True), although this seems unnecessary.