-
Notifications
You must be signed in to change notification settings - Fork 7
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
Allow output types #28
Conversation
I realized that some PR closed #23. So let's continue here. |
I still need to update the examples. But after that we are good to go and have all the features that we wanted. |
Excellent thank you! Let us fully focus on the docs now.
Sent from my T-Mobile 4G LTE Device
…-------- Original message --------
From: Jan-Hendrik Prinz <notifications@github.com>
Date: 3/22/17 5:43 AM (GMT-06:00)
To: markovmodel/adaptivemd <adaptivemd@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Subject: Re: [markovmodel/adaptivemd] [WIP] Allow output types (#28)
I still need to update the examples. But after that we are good to go and have all the features that we wanted.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/markovmodel/adaptivemd","title":"markovmodel/adaptivemd","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/markovmodel/adaptivemd"}},"updates":{"snippets":[{"icon":"PERSON","message":"@jhprinz in #28: I still need to update the examples. But after that we are good to go and have all the features that we wanted."}],"action":{"name":"View Pull Request","url":"#28 (comment)"}}}
|
This reverts commit 3eac97f.
So, examples are up. Please have a look! @nsplattner @thempel @franknoe I think this is much more powerful now. I will update the docs some more and see to make a decent webpage. |
Thank you, will do
Am 22/03/17 um 20:35 schrieb Jan-Hendrik Prinz:
…
So, examples are up. Please have a look! @nsplattner
<https://github.com/nsplattner> @thempel <https://github.com/thempel>
@franknoe <https://github.com/franknoe>
I think this is much more powerful now. I will update the docs some
more and see to make a decent webpage.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#28 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGMeQppWQ7jLjLyIIjymzcBjnivHq5ygks5roXfkgaJpZM4MkkoY>.
--
----------------------------------------------
Prof. Dr. Frank Noe
Head of Computational Molecular Biology group
Freie Universitaet Berlin
Phone: (+49) (0)30 838 75354
Web: research.franknoe.de
Mail: Arnimallee 6, 14195 Berlin, Germany
----------------------------------------------
|
I just tested this PR as described in the tutorial updated in #34. The following happens when I add the engine to the project generators. Did I miss something?
|
This looks like to did not delete the project before restarting. Some of the internals have changed. Try
If that does not help could you post your engine definition? That the message says is that you try to access an attribute ofan object that is marked as stored but does not exist in the db. That could happen if you reuse a pdb file e.g. after deletion of the project. |
Ahh, thanks, I tried this but probably mixed something up. Now this error is resolved, but followed by another one:
|
well, strange... let me see... |
All files were stored 2 before. But only the ones without
This works fine for me. So, I suspect that there is something else getting large. |
Can you compare the file sizes with the original files? Just to make sure there is no overhead? |
This seems to work and also the files on disc show the same number of characters. They have a total size of 11.5 M on disc, so it should be fine.
|
Just scrolled through the above files in my notebook, there content seems fine. Is their anything else being copied? |
This is the question. When exactly did this error happen. I assume you ran the setup from top with PDB, system.xml, etc and then, when storing the engine you got the error? So, it cannot have been caused by some other files, right? There are no other files present. |
Found the bug/storage inefficiency. The file is really stored twice, which is definitely not intended. Will issue a quick fix. Still we should make it use the new storage option |
Wow, this was a real tough one. Involving that No idea, how I found this one. That was probably the most hidden error so far... Still, unfortunately #35 contains the fix and also allowing to store arbitrary large files now. Problem is that when I will merge #35. This one will be merged as well... So let's at least finish this discussion. Additions from #35 are additional features while this PR changes the general concept of trajectories... |
I like the description of this task very much. The only point that concerns me a little bit is the last point (how to featurize), because it creates a relatively hard dependency on PyEMMA and our current naming conventions. There are two issues with this: (1) If you always depend on PyEMMA, this makes the dependencies very heavy (e.g. you also depend on things like matplotlib which are clearly irrelevant for this package) and many dependencies also means there are many ways for the package to break down if dependencies change. (2) Although we don't have a concrete plan for that, it is not impossible that the look+feel of PyEMMA featurization will change at some point. I know there are some deficiencies with the current one. To address that, please check where you actually depend on PyEMMA and if possible find a way to make that dependency optional to your package, i.e. if the user doesn't need a certain functionality (e.g. writes their own analysis class), it shouldn't automatically install PyEMMA. Looking at the examples now... |
merging this |
This implements all of the discussion in #23 .
You can now make arbitrary mixtures of selection/ stride for your engine and run and extend, etc.
Only point missing is that PyEmma will need either a reduced PDB or a selection string to work with not full atom sets (this is because my pyemma script computed backbone angles and without topology this is difficult)
This is pretty neat now. Also intelligent handling of frame numbers etc...
More tomorrow...
New features
output types: An engine has output_types that you can add. These contain information about striding and selections (atom subsets). You can have an arbitrary set of these output_types. Usually you would have a master with full selection and some stride and a subset like
protein
with native strideTrajectory
objects now require aengine
property. I first thought to set this, when you actully run a trajectory, but it makes sense to set this upon creation. The engine contains information about topology and the output types so the trajectory is useless without this information. It also means that you can create the task directly from the trajectory. There are methods.run
and.extend
now for that.Engine
has now two commands.run()
and.extend()
instead of the long names for generating tasks. These are the same for all enginespyemma feature support: This is tricky. I added a way to express pyemma features. What you do is convert the calls of
featurizer.add_[someting](arg1, arg2, ...)
into a dict like{'add_[something]': [arg1, arg2]}
where args can again be calls to the featurizer object. This will allow you basic featurizer construction. If you really need something fancy you have to write your own Analysis class.