-
Notifications
You must be signed in to change notification settings - Fork 4
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
WIP - Parallelization of DistToPoint metrics #217
WIP - Parallelization of DistToPoint metrics #217
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #217 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 37 37
Lines 2646 2707 +61
=========================================
+ Hits 2646 2707 +61 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this. One issue that that there is now a potential circular dependency:
qp.Ensemble.py -> from qp.metrics import quick_moment
qp.metrics.concrete_metric_classes.py -> from qp.ensemble import Ensemble
which means that we can't have
from .concrete_metric_classes import *
in metrics/init.py
We should think a bit about this
It seems like we can clean up the imports in a couple ways. Either would be good. Both would be better in my opinion.
|
I sketched out the UML, and I don't think we have a circular dependency, but I might be wrong about that. I think it's something like this: quick_moment Given the previous comment, I think we can cut the dependency that concrete_metrics takes on Ensemble and potentially the dependency that Ensemble takes on quick_moment. Then the UML would be: quick_moment And Ensemble wouldn't have any metric-related dependencies. |
Yes, to cleaning up the imports as you described. |
I can easily pull out the concrete_metrics Created issue #219 to track the removal of |
…218) * WIP - Initial commit to support writing out a dictionary collection of ensembles. * Implementing a function to read dictionaries of qp ensembles from hdf5 files. * Removing files that were accidentally committed. * Adding test coverage for read and write dict. * Marking test as skipped until the required tables_io work is released. * Unskipping a test now that tables_io has new release. * Adding pragma no cover to value error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!!
Problem & Solution Description (including issue #)
Initial parallelization of PIT metric. Not fully tested yet.
Still need to parallelize CDELoss and Brier.
Code Quality
#pragma: no cover
; in the case of a bugfix, a new test that breaks as a result of the bug has been added