Support CIF #51

mittinatten · 2020-10-24T14:04:44Z

Related to #48. Using gemmi (https://gemmi.readthedocs.io/en/latest/index.html).

Move CLI to be in C++11.
Make PoC.
Make autotools treat C++ properly
Investigate using Gemmi as PDB parser for regular PDBs as well (means the library also needs to be in C++, which in turn could affect Python bindings). - save for later
Investigate cost of/implement support for .gz-variants too. - save for later
Handle insertion codes
Handle (ignore) alternative conformations
Consider using Gemmi's structure class to avoid handling alternative conformations explicitly - save for later
Add changes to documentation

Support all CLI options also for CIF

Testing

Test on large set of PDB codes and make sure the calculation results are the same with .pdb files and .cif files, and that the code doesn't crash, etc.

mittinatten · 2020-10-25T09:42:56Z

History is a bit messy because I moved some changes that were due to changing editor setup to the dev branch in order to keep the diff here only related to the new feature.

* Convert CLI main to C++ * Add GEMMI as submodule

…s as in regular PDB files. Handle insertion codes in CIF files.

mittinatten · 2020-11-28T16:17:12Z

Did some rebasing to make the commits clearer

Addressed AltLoc CIF bug in structure_from_doc

* Add C++ lib to travis packages. * Change travis language to C++. * Test all branches in travis. * Add -lsubunit linking for unit tests. * Comment out unstable test.

… lacking freesasa_structure building implementation.

…parate-models

…ication; unable to remove duplication of func structure_from_doc with std::is_same, need to try something else.

…separate-models PR

…ty; tests are still needed.

Fix allocation issues in node_atom

Output CIF

danny305 · 2021-04-30T12:36:54Z

Super excited! Let me know if you encounter any bugs you need me to address!

…d files (using --output option).

…aces.

mittinatten · 2021-05-05T18:13:27Z

src/cif.cc

+    friend std::ostream &operator<<(std::ostream &os, const freesasa_MCRA &mcra)
+    {
+        os << "Atom(" << mcra._model << " " << mcra._chain << " " << mcra._res_num << " [" << mcra._residue << "] " << mcra._atom << ")";
+        return os;
+    }


I assume this was only used for debugging?

I was initially for debugging but allows for a nice output of the atom. You can keep it or drop it. I don't think its currently being used anywhere.

mittinatten · 2021-05-05T18:16:01Z

So I'm going through everything one last time, and have committed a few minor things. A few things are stylistic. And I found a better solution for handling output. There is an option --output to specify another file than stdout. So I pass a filename instead of a file pointer to the CIF output function, see d609021.

I noticed we are missing docs for CIF output, so I'll fix that too.

…alues available or not.

danny305 · 2021-05-05T18:41:59Z

Awesome! yeah I originally wrote it to output to a file so I am glad you added that additional capability back.

mittinatten · 2021-05-05T18:42:00Z

I made one other minor change. RSA-values are output regardless of FREESASA_OUTPUT_SKIP_REL, in the case where there is no reference available the output will then look like this, which I think is still useful.

loop_
_freeSASA_rsa.asym_id
_freeSASA_rsa.seq_id
_freeSASA_rsa.comp_id
_freeSASA_rsa.abs_total
_freeSASA_rsa.rel_total
_freeSASA_rsa.abs_side_chain
_freeSASA_rsa.rel_side_chain
_freeSASA_rsa.abs_main_chain
_freeSASA_rsa.rel_main_chain
_freeSASA_rsa.abs_apolar
_freeSASA_rsa.rel_apolar
_freeSASA_rsa.abs_polar
_freeSASA_rsa.rel_polar
A 1 MET 54.393508 ? 19.085046 ? 35.308462 ? 28.483151 ? 25.910357 ?
A 2 GLN 74.212106 ? 70.132368 ? 4.079738 ? 11.657433 ? 62.554673 ?
...

danny305 · 2021-05-05T18:43:10Z

Yeah I made it output RSA regardless bc why not. Its a separate block if you don't need it then ignore it. If values are missing then they will be either ? or .. Which is standard for cif.

I don't see the change you made?

mittinatten · 2021-05-05T18:46:41Z

See c204ce7

danny305

So one thing that is missing from all this is where you specify the unit of the freeSASA value. In your original output there is a unit and I did not know where to place that in the output? Maybe it should go in the docs?

mittinatten · 2021-05-05T18:50:15Z

Yeah I thought about that too. I think the docs probably makes the most sense, otherwise we would need to add it to all the fields, I assume: _freeSASA_rsa.abs_side_chain_AA2 or _freeSASA_rsa.abs_side_chain_Angstrom^2, or something, which looks kind of strange.

danny305 · 2021-05-05T18:51:29Z

Yeah it looks super clunky. I think the docs is a better place.

mittinatten · 2021-05-05T18:52:08Z

I'm done for the day, but think we're really close now!

mittinatten · 2021-05-05T18:54:04Z

One more thing, if you can think of any CLI tests that we can use to check that everything works and catch regressions later that would be great. At least we should have one simple case where we compare with a stored CIF output so that we get an error if future modifications changes the format by accident.

danny305 · 2021-05-05T18:55:16Z

Im not sure I understand? Gemmi writes out the format and enforces it? if we did not follow the cif format then gemmi would error out? Could you elaborate/clarify exactly the kind of test you want?

mittinatten · 2021-05-09T13:43:47Z

I was simply thinking of one or more test that runs through the different branches of the CIF output code, so that if something breaks we catch it automatically. For instance if the freesasa_node interface changes somehow in the future, the test suite should trigger an error also for CIF output, to avoid regressions. Also we should check that illegal CLI options are caught with an error message, i.e. --format=cif 1234.pdb and --format=pdb --cif, etc.

Add more regression tests for CIF-related CLI options.

mittinatten · 2021-05-09T14:18:39Z

I added a few, that's probably enough for now.

…ch for the same purpose.

mittinatten · 2021-05-09T14:47:08Z

I've been through the code enough times now to be pretty sure this is ok to publish. Thanks so much for your effort, really nice to collaborate!

I will merge it to the dev-branch and add some version information, etc, before merging to master. As mentioned I will release it as 2.1-beta to allow it to mature a little bit more. That means anyone who clones the repository will get the new features, but they won't be in the published docs yet, and we are still free to make minor changes to for example CLI options, etc, if we realize something is inconsistent in the interface.

mittinatten mentioned this pull request Oct 24, 2020

Support for CIF format #48

Closed

mittinatten force-pushed the feature/cif branch from 61c0123 to b0575f5 Compare October 25, 2020 09:24

mittinatten force-pushed the dev branch from 60f42bd to bb70919 Compare November 28, 2020 16:09

mittinatten added 2 commits November 28, 2020 17:14

Add CIF support to CLI.

738b998

* Convert CLI main to C++ * Add GEMMI as submodule

Use the same logic for ignoring alternative conformations in CIF file…

1e29035

…s as in regular PDB files. Handle insertion codes in CIF files.

mittinatten force-pushed the feature/cif branch from 165524b to 1e29035 Compare November 28, 2020 16:16

mittinatten and others added 22 commits January 5, 2021 19:10

Support --join-models for CIF files and exclude hydrogens correctly.

7206387

Add support for --chain-groups option for CIF files.

b215f29

Add .deps to clean target.

58f5de8

Patched AltLoc bug in cif atom filter.

664c5ba

Cleaned atom altloc filter code.

f4bd648

Merge: Adding fix for autotools-compilation with main.cc

1421bbb

Updated configure.ac to c++14; required for std::make_unique()

7338451

Merge pull request #52 from danny305/feature/cif

5333706

Addressed AltLoc CIF bug in structure_from_doc

Update git submodules to use https instead of git protocol..

73d49b2

Update travis setup to use ubuntu 16.

2245065

Fix/travis c++ (#54)

d1fd5a3

* Add C++ lib to travis packages. * Change travis language to C++. * Test all branches in travis. * Add -lsubunit linking for unit tests. * Comment out unstable test.

Implemented flow control for --separate-chains and --separate-models;…

b3fb1d5

… lacking freesasa_structure building implementation.

populated freesasa_structure with data for --separate-chains and --se…

640eda8

…parate-models

Refactored --separate-models and --separate-chains; removed code dupl…

3d3c87c

…ication; unable to remove duplication of func structure_from_doc with std::is_same, need to try something else.

Solved freesasa_warn linking error.

d7e27de

Fixed memory error for --chain-groups.

879e93e

Fixed --separate-chains and --separate-models incompatibility.

ad14153

Addressed incompatability and formatting for --separate-chains and --…

fd9ce11

…separate-models PR

Added pdb/cif files to test --separate-[chains,models] co-compatibili…

2f564d2

…ty; tests are still needed.

Removed merge boilerplate causing travis failure.

1b0f844

Added dataset of 2000 sequence diversed proteins

33b4d2a

Removed deprecated code from structure_from_doc()

026687f

danny305 and others added 3 commits April 29, 2021 11:31

Merge pull request #2 from mittinatten/fix/atom-node

133c426

Fix allocation issues in node_atom

Refactored/cleaned up code. Ready for final test and merge.

61c9e20

Merge pull request #59 from danny305/feature/cif

cc4251d

Output CIF

mittinatten added 3 commits May 5, 2021 19:56

Pass filename instead to CIF export to allow writing to user specifie…

d609021

…d files (using --output option).

Omit threads information from CIF output.

ef8104d

Make all internal functions in cif.cc static and short names a few pl…

084b22b

…aces.

mittinatten commented May 5, 2021

View reviewed changes

mittinatten added 2 commits May 5, 2021 20:34

Add docs for CIF output.

b402878

Output RSA values in CIF output regardless of if there are relative v…

c204ce7

…alues available or not.

Close file when done in CIF output.

48a36eb

danny305 reviewed May 5, 2021

View reviewed changes

Remove debug code.

23b5e5e

Add check for illegal CLI options and opposite.

95f07fd

Add more regression tests for CIF-related CLI options.

Change some asserts to return FREESASA_FAIL and add catch all try/cat…

a9adc25

…ch for the same purpose.

mittinatten merged commit 2c4a002 into dev May 9, 2021

mittinatten deleted the feature/cif branch October 28, 2021 19:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support CIF #51

Support CIF #51

mittinatten commented Oct 24, 2020 •

edited

mittinatten commented Oct 25, 2020

mittinatten commented Nov 28, 2020

danny305 commented Apr 30, 2021

mittinatten May 5, 2021

danny305 May 5, 2021

mittinatten commented May 5, 2021

danny305 commented May 5, 2021 •

edited

mittinatten commented May 5, 2021

danny305 commented May 5, 2021 •

edited

mittinatten commented May 5, 2021

danny305 left a comment

mittinatten commented May 5, 2021

danny305 commented May 5, 2021

mittinatten commented May 5, 2021

mittinatten commented May 5, 2021

danny305 commented May 5, 2021

mittinatten commented May 9, 2021

mittinatten commented May 9, 2021

mittinatten commented May 9, 2021

Support CIF #51

Support CIF #51

Conversation

mittinatten commented Oct 24, 2020 • edited

mittinatten commented Oct 25, 2020

mittinatten commented Nov 28, 2020

danny305 commented Apr 30, 2021

mittinatten May 5, 2021

Choose a reason for hiding this comment

danny305 May 5, 2021

Choose a reason for hiding this comment

mittinatten commented May 5, 2021

danny305 commented May 5, 2021 • edited

mittinatten commented May 5, 2021

danny305 commented May 5, 2021 • edited

mittinatten commented May 5, 2021

danny305 left a comment

Choose a reason for hiding this comment

mittinatten commented May 5, 2021

danny305 commented May 5, 2021

mittinatten commented May 5, 2021

mittinatten commented May 5, 2021

danny305 commented May 5, 2021

mittinatten commented May 9, 2021

mittinatten commented May 9, 2021

mittinatten commented May 9, 2021

mittinatten commented Oct 24, 2020 •

edited

danny305 commented May 5, 2021 •

edited

danny305 commented May 5, 2021 •

edited