Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Reworked CalculateProjectSASA.py #338

Open
wants to merge 1,252 commits into
base: master
Choose a base branch
from
Open

Conversation

schwancr
Copy link
Contributor

@schwancr schwancr commented Feb 7, 2014

Basically it doesn't make sense to save a single file for an entire dataset because the result of mdtraj.geometry.sasa.shrake_rupley is an array with SASA for each ATOM in the trajectory. So the script will save a file for each trajectory in an input range. Additionally, I removed the atom indices flag because I think most people will use it incorrectly.

…res passing the ProjectInfo.yaml file as well as the stride used in clustering since the shape of the assignments file depends on the trajectory lengths in the project info
…xist. The builder actually assumed that the error was a corrupt xtc file, and so it was stuck in an infinite loop trying to throw away the last frame in the xtc.
… index to save to. Also some other minor changes
… index to save to. Also some other minor changes
…g to read the first file in an empty list. I added a check in the while corrupted loop that will break when there are no files left. This results in a RuntimeError, which is caught by both _update_traj and _load_new_files and correctly just does not add a trajectory to the project. Before the entire script would crash because this error was not caught correctly. I did change it from ValueError, but I have no idea what would be raising that error. This is at the very least more transparent, but some xtc error may still cause a crash.
…to load the pdb from disk for every trajectory
…an discard redundant frames in a reasonable manner. The trouble is, the code that is in there now DOESN'T WORK...
@schwancr
Copy link
Contributor Author

schwancr commented Feb 7, 2014

Personally I think it's better to save a lot of data to disk (especially when your dataset is huge) and then analyze that data after the fact.

i.e. Save the SASA for each atom to disk, and then analyze those results for the total Trp SASA, for instance.


if __name__ == '__main__':
parser = arglib.ArgumentParser(description="""Calculates the Solvent Accessible Surface Area
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put the parser declaration outside of if __name__ == '__main__? This is how the msmb script is able to collect all the parsers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea I didn't realize that

On Fri, Feb 7, 2014 at 2:39 PM, Robert McGibbon [email protected]:

In scripts/CalculateProjectSASA.py:

if name == 'main':

  • parser = arglib.ArgumentParser(description="""Calculates the Solvent Accessible Surface Area

Can you put the parser declaration outside of if name == 'main?
This is how the msmb script is able to collect all the parsershttps://github.com/SimTk/msmbuilder/blob/master/scripts/msmb#L37

Reply to this email directly or view it on GitHubhttps://github.com//pull/338/files#r9558142
.

trajectory""", default='sasa')
parser.add_argument('which', help="""which trajectories to calculate the SASA for.
This script saves a separate file for each trajectory.""", default=[0, np.inf],
nargs=2, type=int)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't supposed to be a list of the indices you want to use, its the start and stop indices. Maybe change the help text to something like:

help="Indices of the initial and final trajectories on which to run SASA calculation. This script saves separate SASA files for each trajectory. Example: '--which 0 10' will calculate the SASA only for the first 10 trajectories in your project."

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe they should just be two separate arguments...

@kyleabeauchamp
Copy link
Contributor

I think this docstring sounds like we're outputting just a single HDF file:

Calculates the Solvent Accessible Surface Area
of all atoms in a subset of the trajectories, or for all trajectories in the project. The
output is a hdf5 file which contains the SASA for each atom in each frame
in each trajectory (or the single trajectory you passed in.

Otherwise looks ok

@schwancr
Copy link
Contributor Author

Yea I didn't change it from the last version. Will do


Sent from Mailbox for iPhone

On Tue, Feb 11, 2014 at 7:10 AM, kyleabeauchamp [email protected]
wrote:

I think this docstring sounds like we're outputting just a single HDF file:

Calculates the Solvent Accessible Surface Area
of all atoms in a subset of the trajectories, or for all trajectories in the project. The
output is a hdf5 file which contains the SASA for each atom in each frame
in each trajectory (or the single trajectory you passed in.

Otherwise looks ok

Reply to this email directly or view it on GitHub:
#338 (comment)

@rmcgibbo
Copy link
Contributor

bump.

@schwancr
Copy link
Contributor Author

This is going to be a pain to merge because of the divergent commit history issues. Any idea as to the best way to do it?

@rmcgibbo
Copy link
Contributor

Get the patchfile and then reapply it on a new branch checked out from
master?

On Sat, Mar 29, 2014 at 12:38 PM, Christian Schwantes <
[email protected]> wrote:

This is going to be a pain to merge because of the divergent commit
history issues. Any idea as to the best way to do it?

Reply to this email directly or view it on GitHubhttps://github.com//pull/338#issuecomment-39006452
.

merge in all of the new msmbuilder changes with merge issues being dealt with by picking SimTk's versions
…ese files won't have their history tracked in the versions in MSMBuilder/
@schwancr
Copy link
Contributor Author

Alright in case anyone was curious, you can use the -X flag with a value of theirs when doing git pull or git merge to just accept all of the remote's changes.

One issue was I needed to delete the src/ directory, which might mean that the library files in MSMBuilder/ are not the same as their previous versions (so their history may be gone forever!)

@rmcgibbo
Copy link
Contributor

I don't think anything is gone forever.

On Sat, Mar 29, 2014 at 1:11 PM, Christian Schwantes <
[email protected]> wrote:

Alright in case anyone was curious, you can use the -X flag with a value
of theirs when doing git pull or git merge to just accept all of the
remote's changes.

One issue was I needed to delete the src/ directory, which might mean
that the library files in MSMBuilder/ are not the same as their previous
versions (so their history may be gone forever!)

Reply to this email directly or view it on GitHubhttps://github.com//pull/338#issuecomment-39007411
.

@schwancr
Copy link
Contributor Author

Fair enough. It's a bit annoying that github thinks that I made 1250 commits because of merging the new msmb

I guess it's a good way to pad my stats, but it just adds a lot of bloat to our commit history

@schwancr
Copy link
Contributor Author

Did I do something wrong? I don't really understand why this happened

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants