Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implied timescales plot #200

Merged
merged 28 commits into from
Jan 31, 2022
Merged

Implied timescales plot #200

merged 28 commits into from
Jan 31, 2022

Conversation

clonker
Copy link
Member

@clonker clonker commented Jan 30, 2022

No description provided.

@clonker clonker requested a review from thempel January 30, 2022 13:52
@codecov
Copy link

codecov bot commented Jan 30, 2022

Codecov Report

Merging #200 (2acd930) into main (78b94b3) will increase coverage by 0.03%.
The diff coverage is 93.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #200      +/-   ##
==========================================
+ Coverage   93.57%   93.61%   +0.03%     
==========================================
  Files         131      134       +3     
  Lines       10544    10644     +100     
==========================================
+ Hits         9867     9964      +97     
- Misses        677      680       +3     
Impacted Files Coverage Δ
deeptime/plots/chapman_kolmogorov.py 0.00% <0.00%> (ø)
deeptime/util/_validation.py 90.69% <ø> (ø)
deeptime/util/stats.py 95.55% <87.50%> (+0.17%) ⬆️
.../estimation/dense/tmat_sampling/tmatrix_sampler.py 91.89% <88.88%> (+1.89%) ⬆️
deeptime/plots/implied_timescales.py 97.40% <97.40%> (ø)
deeptime/__init__.py 100.00% <100.00%> (ø)
deeptime/data/_drunkards_walk_simulator.py 95.37% <100.00%> (ø)
deeptime/decomposition/_koopman.py 91.60% <100.00%> (+0.10%) ⬆️
deeptime/decomposition/_vamp.py 94.52% <100.00%> (ø)
deeptime/markov/_base.py 99.20% <100.00%> (+1.68%) ⬆️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78b94b3...2acd930. Read the comment docs.

self._lagtimes = np.asarray(lagtimes, dtype=int)
self._its = np.asarray(its)
assert self.its.ndim == 2 and self.its.shape[0] == self.n_lagtimes, \
"its should be of shape (lagtimes, processes)."
Copy link
Collaborator

Choose a reason for hiding this comment

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

good to check. it will fail if supplied ITS cover a different number of states, i.e., if some states drop out of the active set.

Copy link
Member Author

Choose a reason for hiding this comment

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

its is now assumed to be a list of timescales per lagtime and this check only looks for the number of lagtimes

@property
def n_processes(self) -> int:
r""" Number of processes. """
return self.its.shape[1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

property not defined if different numbers of processes are covered for different lag times, i.e., due to active set size differences. maybe need an n_processes_max`?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

its = []
its_stats = [] if is_bayesian else None

for model in models:
Copy link
Collaborator

Choose a reason for hiding this comment

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

One could check for missing processes and pad arrays here.

Copy link
Member Author

Choose a reason for hiding this comment

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

added padding in the ctor call

@clonker clonker merged commit b49043b into deeptime-ml:main Jan 31, 2022
@clonker clonker deleted the plots branch January 31, 2022 16:47
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.

2 participants