WIP: Dcd cython #929

kain88-de · 2016-08-08T07:02:36Z

Superseeded by #1155

Fixes #659

Changes made in this Pull Request:

rewrite of dcd module in cython

This is a start to completely rewrite the DCD file handling in cython. It will get rid of the convoluted python/C class mixture we have right now and instead replace all DCD file handling in a cleanly wraped DCDFile object. DCDFile is an object that acts similar to normal filehandle in python similar to XTCFile and TRRFile. This should also help in #926.

At the current stage it is only possible to open/close files.

TODO Cython DCD Wrapper

TODO Update MDAnalysis DCD formats based readers

DCDReader
use generic Reader test for DCDReader

PR Checklist

Tests?
Docs?
CHANGELOG updated?
Issue raised/referenced?

richardjgowers · 2016-08-08T12:41:14Z

package/MDAnalysis/lib/formats/libdcd.pyx

+        # always propagate exceptions forward
+        return False
+
+    def open(self, filename, mode='r'):


Is it possible to make this seek like a file object? That's required for my 239 approach

I wanted to make it seek like the XDRFile. It is on the list to add.

would that be enough for you or do you need more seek functionality?

I'm just reading through. I'd prefer if seek was what you call bytes_seek
else I'm going to have to have 2 versions of everything though.

On Mon, 8 Aug 2016, 14:20 Max Linke, [email protected] wrote:

In package/MDAnalysis/lib/formats/libdcd.pyx
#929 (comment):

self.open(self.fname, mode)

def dealloc(self):

self.close()

def enter(self):

"""Support context manager"""

return self

def exit(self, exc_type, exc_val, exc_tb):

"""Support context manager"""

self.close()

# always propagate exceptions forward

return False

def open(self, filename, mode='r'):

would that be enough for you or do you need more seek functionality?

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/MDAnalysis/mdanalysis/pull/929/files/baacf8a81a69438abce49716167ff7e281bbec4a#r73873127,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AI0jB-PKn7kk-nSDAlcD--tdaMBY_hjGks5qdy0DgaJpZM4Jewd6
.

Will also need a tell method if that's possible

On Mon, 8 Aug 2016, 14:21 Richard Gowers, [email protected] wrote:

I'm just reading through. I'd prefer if seek was what you call bytes_seek
else I'm going to have to have 2 versions of everything though.

On Mon, 8 Aug 2016, 14:20 Max Linke, [email protected] wrote:

In package/MDAnalysis/lib/formats/libdcd.pyx
#929 (comment):

self.open(self.fname, mode)

def dealloc(self):

self.close()

def enter(self):

"""Support context manager"""

return self

def exit(self, exc_type, exc_val, exc_tb):

"""Support context manager"""

self.close()

# always propagate exceptions forward

return False

def open(self, filename, mode='r'):

would that be enough for you or do you need more seek functionality?

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/MDAnalysis/mdanalysis/pull/929/files/baacf8a81a69438abce49716167ff7e281bbec4a#r73873127,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AI0jB-PKn7kk-nSDAlcD--tdaMBY_hjGks5qdy0DgaJpZM4Jewd6
.

Like I wrote this should be close to a normal file object. So it will also receive a tell.

jdetle · 2016-08-08T16:31:18Z

So in #916 it came up that there should be a fastio method to read backwards across a DCD file. This would allow for easy negative stepping in the timeseries function. Should this be addressed here or elsewhere? Also I hope you're aware of the work already done by @arose on porting the DCD c files to python3 objects. https://github.com/arose/simpletraj/tree/master/simpletraj/dcd

kain88-de · 2016-08-08T16:49:09Z

I haven't looked much into the API yet. But negative stepping should be possible with a random seek function. Which this class should get like the XDRFile class has.

About the fork atom @arose. I'm aware of that but I rather have a clean start then the messy dcd reader we have right now. The cython version will also be easier to maintain in the long run.

jbarnoud · 2016-08-20T19:52:08Z

package/MDAnalysis/lib/formats/include/correl.h

+      N. Michaud-Agrawal, E. J. Denning, T. B. Woolf, and
+      O. Beckstein. MDAnalysis: A Toolkit for the Analysis of
+      Molecular Dynamics Simulations. J. Comput. Chem. 32 (2011), 2319--2327,
+      in press.


Out of scope of the PR, but the citation is out of date.

See also MDAnalysis/MDAnalysis.github.io#27 – you could open a new issue for a new comment header on all files and we can discuss there what we want to put into the new header.

And now see #993.

This is a good wirst step

[skip ci]

and fixes other issue I set by accident

kain88-de · 2016-10-02T15:51:32Z

See #187 #475 and mdtraj:#1163

tylerjereddy · 2016-10-19T15:51:07Z

@kain88-de Is there any way to break the remaining work into manageable chunks? Like, you need a cdef function that can do X or Y, accepting these arguments, and returning this value? If I / others could potentially write a few cython functions, PR them to this branch pending your revisions, etc., that might help you focus on the tougher parts.

My Cython skills are getting a little more solid, but the interaction between the pyx file and the c code here is a bit overwhelming at first glance, making it difficult to identify specific tasks that need doing, though I suppose one could just try various .dcd file handling tasks to see what is missing.

kain88-de · 2016-10-19T16:43:40Z

I've updated the first post with a more detailed todo list. The wrapper can now read frames and seek in the dcd file. tests for that would be nice. You can check the libmdaxdr as an example.

https://github.com/MDAnalysis/mdanalysis/blob/develop/testsuite/MDAnalysisTests/formats/test_libmdaxdr.py

note
I'm currently not doing any operations on the dimensions read from the dcd file. This is on purpose. It will allow the wrapper to handle more dcd files and we can implement the whole dimension unpacking that varies from charm and other tools at one place in the python code. Right now it is split between python and C.

kain88-de · 2016-10-19T16:46:11Z

but the interaction between the pyx file and the c code here is a bit overwhelming at first glance

Yes it's not going to be the short and good looking wrapper I anticipated. But if you have any questions about the code. Just leave a comment.

tylerjereddy · 2016-10-23T17:22:42Z

package/MDAnalysis/lib/formats/libdcd.pyx

+
+    def tell(self):
+        """Get current frame"""
+        return self.current_frame


Looks like you already defined the tell() method above? Sorry -- I should have noticed that, but I just realized that my test was passing (once you told me about tell) before I fetched your update to this branch.

OK I removed the last commit again

…e handling interface.

…dimensions.

…_read_write_mode_file and test_open_wrong_mode.

tylerjereddy · 2017-01-10T15:58:03Z

Currently when I run nosetests test_libdcd.py in python 3.5 I get the following issue:

======================================================================
ERROR: Failure: ImportError (No module named 'c_distances_openmp')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/treddy/.local/lib/python3.5/site-packages/nose-1.3.7-py3.5.egg/nose/failure.py", line 39, in runTest
    raise self.exc_val.with_traceback(self.tb)
  File "/Users/treddy/.local/lib/python3.5/site-packages/nose-1.3.7-py3.5.egg/nose/loader.py", line 418, in loadTestsFromName
    addr.filename, addr.module)
  File "/Users/treddy/.local/lib/python3.5/site-packages/nose-1.3.7-py3.5.egg/nose/importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/Users/treddy/.local/lib/python3.5/site-packages/nose-1.3.7-py3.5.egg/nose/importer.py", line 94, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/Users/treddy/anaconda/envs/py35/lib/python3.5/imp.py", line 234, in load_module
    return load_source(name, filename, file)
  File "/Users/treddy/anaconda/envs/py35/lib/python3.5/imp.py", line 172, in load_source
    module = _load(spec)
  File "<frozen importlib._bootstrap>", line 693, in _load
  File "<frozen importlib._bootstrap>", line 673, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 662, in exec_module
  File "<frozen importlib._bootstrap>", line 222, in _call_with_frames_removed
  File "/Users/treddy/python_modules/MDAnalysis/MDA_dev/mdanalysis/testsuite/MDAnalysisTests/formats/test_libdcd.py", line 8, in <module>
    from MDAnalysis.lib.formats.libdcd import DCDFile
  File "/Users/treddy/.local/lib/python3.5/site-packages/MDAnalysis-0.16.0.dev0-py3.5-macosx-10.5-x86_64.egg/MDAnalysis/__init__.py", line 158, in <module>
    from .lib import log
  File "/Users/treddy/.local/lib/python3.5/site-packages/MDAnalysis-0.16.0.dev0-py3.5-macosx-10.5-x86_64.egg/MDAnalysis/lib/__init__.py", line 30, in <module>
    from . import distances  # distances relies on mdamath
  File "/Users/treddy/.local/lib/python3.5/site-packages/MDAnalysis-0.16.0.dev0-py3.5-macosx-10.5-x86_64.egg/MDAnalysis/lib/distances.py", line 115, in <module>
    from c_distances_openmp import OPENMP_ENABLED as USED_OPENMP
ImportError: No module named 'c_distances_openmp'

Ideas? Is there an openmp issue with python 3 for MDA at the moment?

tylerjereddy · 2017-01-10T16:03:06Z

I note that this happen on import of MDAnalysis in py35 as well, even though the install appears to work properly.

richardjgowers · 2017-01-10T16:03:33Z

@tylerjereddy the c_distance_openmp library always gets built, if there's no openmp it just builds it in serial

If you want to rule out openmp completely, you can hack this line in setup.py to has_openmp=False

https://github.com/MDAnalysis/mdanalysis/blob/develop/package/setup.py#L277

kain88-de · 2017-01-10T16:21:39Z

you should be able to set the openmp build in the setup.cfg

tylerjereddy · 2017-01-10T17:07:12Z

The traceback persists even if I apply both approaches, git clean -xfd, and reinstall MDA under python 3.5. Maybe there's some kind of interference on my system from something else despite the usage of a separate conda py35 environment.

jbarnoud · 2017-01-12T14:35:52Z

I get the same traceback when I try to run the tests on python 3. I tried to disable openmp in setup.cfg, but I still get the same error.

richardjgowers · 2017-01-12T14:50:20Z

@jbarnoud looking at that again, you should be able to comment out that USED_OPENMP line and maybe it will work

jbarnoud · 2017-01-12T15:27:39Z

@richardjgowers This seems to work as a workaround. I opened an issue to find a proper fix.

kain88-de added help wanted Python3 Component-Readers Format-DCD refactoring Difficulty-hard Component-Writers Work in progress Component-lib labels Aug 8, 2016

richardjgowers reviewed Aug 8, 2016
View reviewed changes

kain88-de mentioned this pull request Aug 8, 2016

WIP: Reader that does not need open file handles (#239) #926

Closed

jbarnoud mentioned this pull request Aug 17, 2016

Temporarily remove python 3 travis builds, allow osx to fail #943

Merged

jbarnoud reviewed Aug 20, 2016
View reviewed changes

kain88-de force-pushed the dcd-cython branch 2 times, most recently from e5aebd6 to 9bad8e7 Compare August 28, 2016 17:09

kain88-de added 13 commits October 1, 2016 20:59

first dcd stumb cython file

bb13d32

first working darft of dcd reimplmentation

3fda8a8

This is a good wirst step

open and close dcd files

6ee3416

add context manager support

9719b79

[skip ci]

remove pass statement

4d1ec71

correctly handle is_open flag cases

6159a2c

[skip ci]

start dcd read header stub

b4642d2

add include guards for fastio.h

d30d53a

read DCD Header

a36b04f

and fixes other issue I set by accident

free memory when closing

2c14c72

remarks handling

3fb31e5

comment all the things

3f71791

convert remarks to python strings

cc1e380

kain88-de added 3 commits October 4, 2016 07:59

improve reading API

b681465

finish reading and iteration protocol

794c533

add seek support

629b854

kain88-de force-pushed the dcd-cython branch from 5328835 to 629b854 Compare October 4, 2016 08:23

tylerjereddy mentioned this pull request Oct 21, 2016

[WIP] TST: Draft of testing module for new Cython-based DCD file handling #1046

Merged

tylerjereddy reviewed Oct 23, 2016

View reviewed changes

kain88-de force-pushed the dcd-cython branch from 3355a37 to 629b854 Compare October 23, 2016 17:44

tylerjereddy added 4 commits October 24, 2016 22:00

TST: Added first draft of testing module for new Cython-based DCD fil…

65926f6

…e handling interface.

TST: All libdcd unit tests now pass, except one related to unit cell …

5d669c5

…dimensions.

TST: Added several more unit tests to test_libdcd.py.

111c44a

TST: updated unit tests to reflect appropriate Error raising for test…

78a4257

…_read_write_mode_file and test_open_wrong_mode.

kain88-de mentioned this pull request Nov 13, 2016

Python 3.5 through Miniconda in Windows10 visual studio 2015 #1071

Closed

kain88-de mentioned this pull request Nov 27, 2016

WIP: issue 659: DCD Python 3 #682

Closed

tylerjereddy mentioned this pull request Jan 10, 2017

[WIP] DCD Cython Changes #1155

Closed

jbarnoud mentioned this pull request Jan 12, 2017

Cannot import on python3: missing c_distances_openmp #1157

Closed

kain88-de closed this Feb 3, 2017

kain88-de deleted the dcd-cython branch January 24, 2018 19:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Dcd cython #929

WIP: Dcd cython #929

kain88-de commented Aug 8, 2016 •

edited

Loading

richardjgowers Aug 8, 2016

kain88-de Aug 8, 2016

kain88-de Aug 8, 2016

richardjgowers Aug 8, 2016

richardjgowers Aug 8, 2016

kain88-de Aug 8, 2016

jdetle commented Aug 8, 2016

kain88-de commented Aug 8, 2016

jbarnoud Aug 20, 2016

orbeckst Aug 29, 2016

orbeckst Sep 20, 2016

kain88-de commented Oct 2, 2016

tylerjereddy commented Oct 19, 2016

kain88-de commented Oct 19, 2016

kain88-de commented Oct 19, 2016

tylerjereddy Oct 23, 2016

kain88-de Oct 23, 2016

tylerjereddy commented Jan 10, 2017

tylerjereddy commented Jan 10, 2017

richardjgowers commented Jan 10, 2017

kain88-de commented Jan 10, 2017

tylerjereddy commented Jan 10, 2017

jbarnoud commented Jan 12, 2017

richardjgowers commented Jan 12, 2017

jbarnoud commented Jan 12, 2017

WIP: Dcd cython #929

WIP: Dcd cython #929

Conversation

kain88-de commented Aug 8, 2016 • edited Loading

TODO Cython DCD Wrapper

TODO Update MDAnalysis DCD formats based readers

PR Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdetle commented Aug 8, 2016

kain88-de commented Aug 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kain88-de commented Oct 2, 2016

tylerjereddy commented Oct 19, 2016

kain88-de commented Oct 19, 2016

kain88-de commented Oct 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tylerjereddy commented Jan 10, 2017

tylerjereddy commented Jan 10, 2017

richardjgowers commented Jan 10, 2017

kain88-de commented Jan 10, 2017

tylerjereddy commented Jan 10, 2017

jbarnoud commented Jan 12, 2017

richardjgowers commented Jan 12, 2017

jbarnoud commented Jan 12, 2017

kain88-de commented Aug 8, 2016 •

edited

Loading