search

MolarVerse / PQAnalysis

PQAnalysis is a API/CLI python package for the analysis of MD simulations
https://molarverse.github.io/PQAnalysis/
MIT License
4 stars 2 forks source link

Feature/xyz reader performance #98

Closed 97gamjak closed 1 month ago

97gamjak commented 1 month ago

Found a small error in lodical expressions when reading trajectory files.

Actually it is not an error, some time ago I implemented a constant topology approach for reading files to speed up things. Actually there were to is not None, which should have been is None and therefore also the constant topology approach was treated as it should build a new topology for each frame.

PERFORMANCE SPEEDUP > 12.0 = 1200%

github-actions[bot] commented 1 month ago

PYLINT REPORT

Your code has been rated at 9.52/10 (previous run: 9.52/10, +0.00)

Full report Raw metrics =========== |type |number |% |previous |difference | |----------|-------|------|---------|-----------| |code |7874 |39.92 |7876 |-2.00 | |docstring |8660 |43.90 |8660 |= | |comment |270 |1.37 |268 |+2.00 | |empty |2922 |14.81 |2921 |+1.00 | Duplication =========== | |now |previous |difference | |-------------------------|------|---------|-----------| |nb duplicated lines |0 |0 |0 | |percent duplicated lines |0.000 |0.000 |= | Messages by category ==================== |type |number |previous |difference | |-----------|-------|---------|-----------| |convention |1 |1 |1 | |refactor |51 |51 |51 | |warning |14 |14 |14 | |error |31 |31 |31 | % errors / warnings by module ============================= |module |error |warning |refactor |convention | |--------------------------------------------------------|------|--------|---------|-----------| |PQAnalysis.io.topology_file.topology_file_reader |67.74 |0.00 |0.00 |0.00 | |PQAnalysis.io.nep.nep_writer |12.90 |0.00 |15.69 |100.00 | |PQAnalysis.io.traj_file.frame_reader |6.45 |0.00 |3.92 |0.00 | |PQAnalysis |3.23 |7.14 |0.00 |0.00 | |PQAnalysis.io.input_file_reader.input_file_parser |3.23 |0.00 |1.96 |0.00 | |PQAnalysis.io.traj_file.api |3.23 |0.00 |0.00 |0.00 | |PQAnalysis.io.gen_file.gen_file_reader |3.23 |0.00 |0.00 |0.00 | |PQAnalysis.atomic_system.atomic_system |0.00 |14.29 |11.76 |0.00 | |PQAnalysis.topology.__init__ |0.00 |14.29 |0.00 |0.00 | |PQAnalysis.tools.traj_to_com_traj |0.00 |14.29 |0.00 |0.00 | |PQAnalysis.io.moldescriptor_reader |0.00 |14.29 |0.00 |0.00 | |PQAnalysis.utils.custom_logging |0.00 |7.14 |0.00 |0.00 | |PQAnalysis.io.write_api |0.00 |7.14 |0.00 |0.00 | |PQAnalysis.io.input_file_reader.pq.output_files |0.00 |7.14 |0.00 |0.00 | |PQAnalysis.core.api |0.00 |7.14 |0.00 |0.00 | |PQAnalysis.atomic_system._decorators |0.00 |7.14 |0.00 |0.00 | |PQAnalysis.io.traj_file.trajectory_reader |0.00 |0.00 |9.80 |0.00 | |PQAnalysis.tools.add_molecule |0.00 |0.00 |7.84 |0.00 | |PQAnalysis.atomic_system._properties |0.00 |0.00 |5.88 |0.00 | |PQAnalysis.analysis.rdf.rdf |0.00 |0.00 |5.88 |0.00 | |PQAnalysis.topology.topology |0.00 |0.00 |3.92 |0.00 | |PQAnalysis.topology.bonded_topology.dihedral |0.00 |0.00 |3.92 |0.00 | |PQAnalysis.core.residue |0.00 |0.00 |3.92 |0.00 | |PQAnalysis.atomic_system._standard_properties |0.00 |0.00 |3.92 |0.00 | |PQAnalysis.traj.formats |0.00 |0.00 |1.96 |0.00 | |PQAnalysis.topology.selection |0.00 |0.00 |1.96 |0.00 | |PQAnalysis.topology.bonded_topology.bonded_topology |0.00 |0.00 |1.96 |0.00 | |PQAnalysis.topology.bonded_topology.bond |0.00 |0.00 |1.96 |0.00 | |PQAnalysis.topology.bonded_topology.angle |0.00 |0.00 |1.96 |0.00 | |PQAnalysis.io.restart_file.restart_reader |0.00 |0.00 |1.96 |0.00 | |PQAnalysis.io.input_file_reader.pq_analysis._parse |0.00 |0.00 |1.96 |0.00 | |PQAnalysis.io.input_file_reader.pq.pq_input_file_reader |0.00 |0.00 |1.96 |0.00 | |PQAnalysis.io.info_file_reader |0.00 |0.00 |1.96 |0.00 | |PQAnalysis.io.formats |0.00 |0.00 |1.96 |0.00 | |PQAnalysis.core.cell.cell |0.00 |0.00 |1.96 |0.00 | Messages ======== |message id |occurrences | |--------------------------------|------------| |possibly-used-before-assignment |30 | |too-many-arguments |18 | |too-many-instance-attributes |10 | |fixme |10 | |inconsistent-return-statements |9 | |too-many-locals |3 | |too-complex |3 | |duplicate-code |3 | |unused-import |2 | |too-many-branches |2 | |unused-argument |1 | |too-many-statements |1 | |too-many-return-statements |1 | |too-many-public-methods |1 | |too-many-lines |1 | |no-member |1 | |arguments-differ |1 |
codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 92.59259% with 8 lines in your changes missing coverage. Please review.

Project coverage is 85.78%. Comparing base (9a9bcea) to head (254e3df). Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #98 +/- ## ========================================== + Coverage 80.67% 85.78% +5.11% ========================================== Files 123 125 +2 Lines 4931 5003 +72 ========================================== + Hits 3978 4292 +314 + Misses 953 711 -242 ``` | [Flag](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse) | `85.78% <92.59%> (+5.11%)` | :arrow_up: | | [Files](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse) | Coverage Δ | | |---|---|---| | [PQAnalysis/io/base.py](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98?src=pr&el=tree&filepath=PQAnalysis%2Fio%2Fbase.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse#diff-UFFBbmFseXNpcy9pby9iYXNlLnB5) | `100.00% <100.00%> (ø)` | | | [PQAnalysis/io/topology\_file/\_\_init\_\_.py](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98?src=pr&el=tree&filepath=PQAnalysis%2Fio%2Ftopology_file%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse#diff-UFFBbmFseXNpcy9pby90b3BvbG9neV9maWxlL19faW5pdF9fLnB5) | `100.00% <100.00%> (ø)` | | | [...QAnalysis/io/topology\_file/topology\_file\_writer.py](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98?src=pr&el=tree&filepath=PQAnalysis%2Fio%2Ftopology_file%2Ftopology_file_writer.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse#diff-UFFBbmFseXNpcy9pby90b3BvbG9neV9maWxlL3RvcG9sb2d5X2ZpbGVfd3JpdGVyLnB5) | `90.44% <100.00%> (+59.35%)` | :arrow_up: | | [PQAnalysis/io/traj\_file/frame\_reader.py](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98?src=pr&el=tree&filepath=PQAnalysis%2Fio%2Ftraj_file%2Fframe_reader.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse#diff-UFFBbmFseXNpcy9pby90cmFqX2ZpbGUvZnJhbWVfcmVhZGVyLnB5) | `99.09% <ø> (ø)` | | | [PQAnalysis/io/traj\_file/trajectory\_reader.py](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98?src=pr&el=tree&filepath=PQAnalysis%2Fio%2Ftraj_file%2Ftrajectory_reader.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse#diff-UFFBbmFseXNpcy9pby90cmFqX2ZpbGUvdHJhamVjdG9yeV9yZWFkZXIucHk=) | `99.42% <100.00%> (+0.58%)` | :arrow_up: | | [PQAnalysis/topology/bonded\_topology/\_\_init\_\_.py](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98?src=pr&el=tree&filepath=PQAnalysis%2Ftopology%2Fbonded_topology%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse#diff-UFFBbmFseXNpcy90b3BvbG9neS9ib25kZWRfdG9wb2xvZ3kvX19pbml0X18ucHk=) | `100.00% <100.00%> (ø)` | | | [PQAnalysis/topology/bonded\_topology/angle.py](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98?src=pr&el=tree&filepath=PQAnalysis%2Ftopology%2Fbonded_topology%2Fangle.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse#diff-UFFBbmFseXNpcy90b3BvbG9neS9ib25kZWRfdG9wb2xvZ3kvYW5nbGUucHk=) | `100.00% <100.00%> (ø)` | | | [PQAnalysis/topology/bonded\_topology/bond.py](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98?src=pr&el=tree&filepath=PQAnalysis%2Ftopology%2Fbonded_topology%2Fbond.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse#diff-UFFBbmFseXNpcy90b3BvbG9neS9ib25kZWRfdG9wb2xvZ3kvYm9uZC5weQ==) | `100.00% <100.00%> (ø)` | | | [...alysis/topology/bonded\_topology/bonded\_topology.py](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98?src=pr&el=tree&filepath=PQAnalysis%2Ftopology%2Fbonded_topology%2Fbonded_topology.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse#diff-UFFBbmFseXNpcy90b3BvbG9neS9ib25kZWRfdG9wb2xvZ3kvYm9uZGVkX3RvcG9sb2d5LnB5) | `100.00% <ø> (ø)` | | | [PQAnalysis/topology/bonded\_topology/dihedral.py](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98?src=pr&el=tree&filepath=PQAnalysis%2Ftopology%2Fbonded_topology%2Fdihedral.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse#diff-UFFBbmFseXNpcy90b3BvbG9neS9ib25kZWRfdG9wb2xvZ3kvZGloZWRyYWwucHk=) | `100.00% <100.00%> (ø)` | | | ... and [4 more](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse) | | ... and [6 files with indirect coverage changes](https://app.codecov.io/gh/MolarVerse/PQAnalysis/pull/98/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MolarVerse)
galjos commented 1 month ago

@97gamjak should this be a release pull request? Merging directly to main?

97gamjak commented 1 month ago

no sry

On Wed, 5 Jun 2024, 07:07 Josef M. Gallmetzer, @.***> wrote:

@97gamjak https://github.com/97gamjak should this be a release pull request? Merging directly to main?

— Reply to this email directly, view it on GitHub https://github.com/MolarVerse/PQAnalysis/pull/98#issuecomment-2148864734, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASNGWAQSKK6TLMI4E7UTUJ3ZF2MHXAVCNFSM6AAAAABIZR6WN2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBYHA3DINZTGQ . You are receiving this because you were mentioned.Message ID: @.***>