Skip to content

fix: repair test_ratio splitting and trajectory writing in NEPWriter - #197

Open
galjos wants to merge 1 commit into
devfrom
fix/build-nep-traj
Open

fix: repair test_ratio splitting and trajectory writing in NEPWriter#197
galjos wants to merge 1 commit into
devfrom
fix/build-nep-traj

Conversation

@galjos

@galjos galjos commented Aug 24, 2026

Copy link
Copy Markdown
Member

_setup_frame_splitting_for_training never used a plain test_ratio, so the later division by the derived sum was zero over zero; and write_from_trajectory omitted the file argument, shifting use_forces into the file slot and raising on every call. The ratio is now taken directly when total_ratios is not given, and the file is passed through.

Added one regression test per bug; a 4-frame trajectory with test_ratio=0.25 now yields 3 train and 1 test frames.

Fixes #196.

@galjos
galjos requested a review from 97gamjak August 24, 2026 07:29
@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.08%. Comparing base (0961625) to head (6322198).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #197      +/-   ##
==========================================
+ Coverage   93.94%   96.08%   +2.14%     
==========================================
  Files         177      177              
  Lines        9227     9230       +3     
==========================================
+ Hits         8668     8869     +201     
+ Misses        559      361     -198     
Flag Coverage Δ
unittests 96.08% <100.00%> (+2.14%) ⬆️
Files with missing lines Coverage Δ
PQAnalysis/io/nep/nep_writer.py 78.32% <100.00%> (+65.95%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

_setup_frame_splitting_for_training derived the split ratios only
from total_ratios, so passing test_ratio alone divided by a zero
frame sum and crashed with ZeroDivisionError while the requested
ratio was ignored. The ratios are now taken directly from test_ratio
when total_ratios is not given. write_from_trajectory also omitted
the file argument when calling write_from_atomic_system, shifting
the boolean flags into the file slot so every call raised a type
error; the writer's own file handle is now passed explicitly.
@galjos
galjos force-pushed the fix/build-nep-traj branch from 4109811 to 6322198 Compare August 24, 2026 07:36
@github-actions

Copy link
Copy Markdown
Contributor

PYLINT REPORT

Your code has been rated at 9.78/10

Full report

Raw metrics

type number % previous difference
code 15658 45.62 NC NC
docstring 13049 38.01 NC NC
comment 463 1.35 NC NC
empty 5156 15.02 NC NC

Duplication

now previous difference
nb duplicated lines 0 NC NC
percent duplicated lines 0.000 NC NC

Messages by category

type number previous difference
convention 15 NC NC
refactor 136 NC NC
warning 17 NC NC
error 5 NC NC

% errors / warnings by module

module error warning refactor convention
PQAnalysis.type_checking 40.00 0.00 0.00 0.00
PQAnalysis.atomic_system.atomic_system 20.00 11.76 6.62 0.00
PQAnalysis.analysis.vibrational.vibrational_analysis 20.00 0.00 7.35 0.00
PQAnalysis 20.00 0.00 0.00 0.00
PQAnalysis.analysis.vacf._vacf_kernel_py 0.00 11.76 1.47 0.00
PQAnalysis.tools.traj_to_com_traj 0.00 11.76 0.00 0.00
PQAnalysis.io.traj_file._process_lines_py 0.00 11.76 0.00 0.00
PQAnalysis.io.moldescriptor_reader 0.00 11.76 0.00 0.00
PQAnalysis.tools.add_molecule 0.00 5.88 4.41 0.00
PQAnalysis.analysis.rdf._rdf_kernel_py 0.00 5.88 2.21 0.00
PQAnalysis.analysis.msd._msd_kernel_py 0.00 5.88 2.21 0.00
PQAnalysis.io.conversion_api 0.00 5.88 1.47 0.00
PQAnalysis.version 0.00 5.88 0.00 0.00
PQAnalysis.utils.custom_logging 0.00 5.88 0.00 0.00
PQAnalysis.io.write_api 0.00 5.88 0.00 0.00
PQAnalysis.io.restart_file.restart_writer 0.00 0.00 14.71 0.00
PQAnalysis.analysis.rdf.rdf 0.00 0.00 6.62 6.67
PQAnalysis.analysis.msd.msd 0.00 0.00 5.88 33.33
PQAnalysis.io.nep.nep_writer 0.00 0.00 5.88 6.67
PQAnalysis.analysis.vacf.vacf 0.00 0.00 4.41 13.33
PQAnalysis.io.traj_file._slab_parser_py 0.00 0.00 3.68 0.00
PQAnalysis.analysis.vacf.spectrum 0.00 0.00 3.68 0.00
PQAnalysis.io.traj_file.trajectory_reader 0.00 0.00 2.94 0.00
PQAnalysis.topology.bonded_topology.dihedral 0.00 0.00 2.21 0.00
PQAnalysis.core.residue 0.00 0.00 2.21 0.00
PQAnalysis.topology.bonded_topology.bonded_topology 0.00 0.00 1.47 0.00
PQAnalysis.topology.bonded_topology.bond 0.00 0.00 1.47 0.00
PQAnalysis.topology.bonded_topology.angle 0.00 0.00 1.47 0.00
PQAnalysis.io.traj_file.raw_frame_reader 0.00 0.00 1.47 0.00
PQAnalysis.core.cell.cell 0.00 0.00 1.47 0.00
PQAnalysis.atomic_system._standard_properties 0.00 0.00 1.47 0.00
PQAnalysis.analysis.vacf.api 0.00 0.00 1.47 0.00
PQAnalysis.analysis.spectrum_broadening.api 0.00 0.00 1.47 0.00
PQAnalysis.analysis.momentum.api 0.00 0.00 1.47 0.00
PQAnalysis.analysis.vibrational.vibrational_input_file_reader 0.00 0.00 0.74 6.67
PQAnalysis.traj.formats 0.00 0.00 0.74 0.00
PQAnalysis.topology.topology 0.00 0.00 0.74 0.00
PQAnalysis.topology.selection 0.00 0.00 0.74 0.00
PQAnalysis.io.traj_file.frame_reader 0.00 0.00 0.74 0.00
PQAnalysis.io.restart_file.restart_reader 0.00 0.00 0.74 0.00
PQAnalysis.io.input_file_reader.pq_analysis._parse 0.00 0.00 0.74 0.00
PQAnalysis.io.input_file_reader.pq.pq_input_file_reader 0.00 0.00 0.74 0.00
PQAnalysis.io.input_file_reader.input_file_parser 0.00 0.00 0.74 0.00
PQAnalysis.io.info_file_reader 0.00 0.00 0.74 0.00
PQAnalysis.io.formats 0.00 0.00 0.74 0.00
PQAnalysis.analysis.momentum.momentum 0.00 0.00 0.74 0.00
PQAnalysis.core.atom.element 0.00 0.00 0.00 13.33
PQAnalysis.analysis.vacf.vacf_input_file_reader 0.00 0.00 0.00 6.67
PQAnalysis.analysis.rdf.rdf_input_file_reader 0.00 0.00 0.00 6.67
PQAnalysis.analysis.msd.msd_input_file_reader 0.00 0.00 0.00 6.67

Messages

message id occurrences
too-many-positional-arguments 29
too-many-arguments 29
duplicate-code 20
too-many-locals 15
too-many-instance-attributes 13
invalid-name 12
too-complex 9
inconsistent-return-statements 8
fixme 7
too-many-branches 5
missing-type-doc 4
too-many-statements 3
too-many-return-statements 3
too-many-lines 3
unused-import 2
redefined-builtin 2
missing-kwoa 2
use-set-for-membership 1
unexpected-keyword-arg 1
too-many-public-methods 1
possibly-used-before-assignment 1
no-member 1
missing-param-doc 1
arguments-differ 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant