Add Site.residence_times() method - #46
Merged
Merged
Conversation
Add a method to compute per-atom residence time run lengths from site trajectory data. Includes a filter_length parameter for smoothing out brief excursions before computing run lengths. Closes #44
Runs touching the first or last timestep are now excluded by default (include_edge_runs=False) since they underestimate the true residence time. Filtering no longer fills gaps at the trajectory edges, as this would bias towards longer occupation times.
Add contrasting example showing edge run exclusion behaviour, and document TypeError in Raises section.
There was a problem hiding this comment.
Pull request overview
Adds a Site.residence_times() analysis method to compute per-atom consecutive occupation run lengths from Site.trajectory, along with tests and a new documentation guide describing usage, edge-run handling, and gap filtering.
Changes:
- Implement
Site.residence_times(filter_length=0, include_edge_runs=False)plus_filter_gaps()helper insite_analysis/site.py. - Add comprehensive unit tests for core run extraction, edge-run exclusion, and gap filtering behavior.
- Add a new documentation guide and link it from the docs index.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
site_analysis/site.py |
Implements residence time extraction and short-gap filtering logic. |
tests/test_site.py |
Adds unit tests covering residence time behavior and parameter validation. |
docs/source/index.md |
Adds the new residence time guide to the guides toctree. |
docs/source/guides/residence_times.md |
New guide explaining residence time analysis, edge-run exclusion, and filtering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Document the intended use case for filtering (brief boundary-crossing artefacts), the per-site independence caveat for rapid oscillation patterns, and how heavy filtering dependence can signal poor site definitions.
Residence times follow an exponential distribution in a hopping model, so only the mean is a useful summary statistic.
There was a problem hiding this comment.
Pull request overview
This PR adds a new Site.residence_times() API to compute per-atom consecutive-occupation run lengths from a site’s trajectory, with optional short-gap filtering and default exclusion of trajectory-edge runs to avoid truncated residence times.
Changes:
- Added
Site.residence_times()and helper_filter_gaps()for gap filling and run-length extraction. - Added a comprehensive unit test suite covering edge exclusion and filtering behavior.
- Added a new documentation guide and linked it from the main docs index.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
site_analysis/site.py |
Implements residence_times() and _filter_gaps() for per-atom run-length extraction with optional filtering and edge-run handling. |
tests/test_site.py |
Adds unit tests for empty trajectories, multi-atom behavior, edge-run exclusion, and filtering rules. |
docs/source/index.md |
Adds the new residence time guide to the Guides toctree. |
docs/source/guides/residence_times.md |
New guide describing residence time analysis, edge-run exclusion, and filtering interpretation/limitations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+327
to
+329
| run_lengths: list[int] = [] | ||
| for atom in sorted(all_atoms): | ||
| occupied = np.array([atom in ts for ts in self.trajectory], dtype=bool) |
Comment on lines
+469
to
+472
| """Test same atom visiting site in two separate interior runs.""" | ||
| site = ConcreteSite() | ||
| site.trajectory = [[], [1], [], [1], []] | ||
| self.assertEqual(site.residence_times(), (1, 1)) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Site.residence_times()method that computes per-atom consecutive-occupation run lengths from the site's trajectory datafilter_lengthparameter smooths brief excursions by filling interior gaps of N or fewer consecutive unoccupied frames before computing run lengths. Edge gaps are never filled, as this would always bias towards longer occupation timesinclude_edge_runs=False) because they are truncated by the trajectory boundary and underestimate the true residence timeValueErrorfor negativefilter_lengthandTypeErrorfor non-integer valuesCloses #44