-
Notifications
You must be signed in to change notification settings - Fork 1
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
Total PowerAnalytics Redesign: ComponentSelector
s and Metric
s
#24
Open
GabrielKS
wants to merge
70
commits into
NREL-Sienna:main
Choose a base branch
from
GabrielKS:gks/entity-metric-redesign-psy4
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
70 commits
Select commit
Hold shift + click to select a range
bb3dea6
Implement ComponentEntity
GabrielKS c13c836
Implement ListEntitySet
GabrielKS c7a9729
Implement SubtypeEntitySet
GabrielKS 0bdbd2c
Implement TopologyEntitySet
GabrielKS 37423ae
Implement FilterEntitySet
GabrielKS ee2d43c
Make test simulations more interesting
GabrielKS c352903
Implement the very basics of Metrics
GabrielKS 1fc29ea
Refactor test setup into its own file
GabrielKS 438661a
Implement some input convenience functions
GabrielKS 53809a5
Minor create_problem_results_dict bugfix
GabrielKS b687bd1
Add metrics wrapper functions and compute_all
GabrielKS b0f7b49
Make entities only return available components
GabrielKS 233f439
More gracefully handle missing data (draft)
GabrielKS ea269c0
Add some Metric creation infrastructure and built-in Metrics
GabrielKS 01e2041
Add metadata columns feature
GabrielKS 1c61537
Add aggregate_time function
GabrielKS 1570519
Add hcat_timed and calc_discharge_cycles
GabrielKS 7840602
Add SystemTimedMetrics
GabrielKS 9fe0c33
Add some more built-in metrics
GabrielKS 0bc3769
Add ResultsTimelessMetrics
GabrielKS 04d87d8
Add CustomTimedMetric (untested), more built-in metrics
GabrielKS afbd015
Implement compose_metrics
GabrielKS b8e54ef
Give metrics control over aggregation functions
GabrielKS 4228377
Add more built-in metrics!
GabrielKS 396e95b
Add another way to call compute_all
GabrielKS 4469340
Add aggregation metadata, etc. (see details)
GabrielKS e731c6e
Fix calc_capacity_factor
GabrielKS 36840da
Add author
GabrielKS 214537d
Implement compute_set
GabrielKS 42ec449
Resolve domain knowledge issues in builtin_metrics.jl
GabrielKS 43b5f23
Add documentation for input, metrics
GabrielKS 0e68361
Fix metrics documentation formatting
GabrielKS ed1969c
Fix compute_all kwargs bug
GabrielKS 3d7ed17
Replace get_populated_decision_problem_results
tengis-nrl da4d1ca
run the formatter
tengis-nrl 3fecf8e
Add built-in entities for load, storage, fuel types
GabrielKS 02c72d9
Rename `Entity` to `ComponentSelector`, handle the ensuing carnage
GabrielKS e1c5a7a
Make use of caching in `read_component_result` for massive speedup
GabrielKS dbcd069
Accommodate `psy4` cost function refactor
GabrielKS b150277
Rename Entity-related files to refer to ComponentSelector instead
GabrielKS 028804a
Remove ComponentSelector files for move to IS and PSY
GabrielKS acc3f94
make PA tests pass
tengis-nrl 0f199e5
run formatter
tengis-nrl 3b0e73e
Update storage naming for `psy4`
GabrielKS 7585efc
Reevaluate `ComponentSelector`-related imports and exports
GabrielKS 2b9ee92
Calculate metrics only on components marked available
GabrielKS be6d683
Only `compute` on `available` subselectors, use PSY4 storage interface
GabrielKS d09ee93
Prototype `ComponentSelector` grouping (tests not updated)
GabrielKS ea9e418
Prototype `Metric` grouping and callability (tests not updated)
GabrielKS 2779e9a
Fix bug in calling `repeat`
GabrielKS 50b9ecf
Rename `select_components` to `make_selector`
GabrielKS d195534
Add ability to disable test files like in other Sienna packages
GabrielKS 036f57f
Accommodate upstream refactors
GabrielKS 0245c92
Wrap built-in selectors in their own submodule
GabrielKS edf9e34
Rename `input.jl` to `input_utils.jl`
GabrielKS ac386ee
Cleanup: move helper functions to `output_utils.jl`, use `@kwdef`
GabrielKS ee503dc
Use `kwargs...` instead of `start_time, len`
GabrielKS ba5dc48
Remove `Metric` `description` field in favor of docstrings
GabrielKS f27bfdd
Cleanup: move more code to `utils` files, run formatter
GabrielKS 889aad9
Make minor adjustments, finish `compute` interface change
GabrielKS 5569eaf
Prototype `Metrics` submodule
GabrielKS 818ee94
Replace `with_component_agg_fn`, etc. with `rebuild_metric`
GabrielKS c43c5e4
Make tests pass, add necessary imports, adjust documentation
GabrielKS d0c7e1f
Add tests for built-in metrics; fix bugs thereby uncovered
GabrielKS 7978528
Accommodate `filterby` -> `scope_limiter` rename
GabrielKS db54809
Bugfix: add fallback for when `get_available` is not defined
GabrielKS a512703
Minor edits from PR comments 1
GabrielKS b37a78d
Accommodate `ComponentSelector` API changes
GabrielKS e139fa9
Refactor `generator_mapping.yaml`, prototype new built-in selectors
GabrielKS 3173796
Clean up `generator_mapping.yaml` selectors implementation, test
GabrielKS File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Oops, something went wrong.
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.
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.
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.
My impression was that
hasmethod
isn't really supposed to be used in production code like this (see e.g. here) but happy to switch if the consensus is it's less bad than the try/catchThere 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.
It's an exported function with proper docstrings! I'm not sure about that comment. Anyway, we could implement
get_available(x::Component) = true
. I'll defer to @jd-lara if that is a reasonable default. It has the same outcome here.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.
While hasmethod certainly isn't ideal, it is faster than try/catch: https://discourse.julialang.org/t/performance-of-hasmethod-vs-try-catch-on-methoderror/99827.
I agree with what that commenter would likely say...this is ill-formed dynamism. A default get_available is obviously still better.
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.
Agree that the best option for this use case is to define
get_available
on allComponent
s — @jd-lara does that work for you?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.
Without objection, moving forward with defining
in PSY to eliminate this.