-
Notifications
You must be signed in to change notification settings - Fork 62
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] Structuring 3 #760
Open
ardunn
wants to merge
46
commits into
TRI-AMDD:master
Choose a base branch
from
ardunn:structure3
base: master
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
[WIP] Structuring 3 #760
Conversation
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
…trs can be accessed for single items
…trs can be accessed for single items
…level and Cycle-level
…dd legacy conversions
…is field usually signifies (it is generally not an index)
…tep, MultiStep, and Cycle
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.
Structuring 3
A rewriting of the structuring class to make BEEP structuring more flexible, efficient, and idiomatic.
main contributions
Core implementations of
Step
,Cycle
, andRun
to replaceBEEPDatapath
step_type
->step_label
) and cycles (cycle_label
). Useful for working with the resulting dataframes.Run
children (closes Validation and conversion schema can be moved entirely into Datapath children #390)indeterminate_step_charge_default
needs test #757, closes Includestep_type_name
in structured data #660)Comprehensive configuration of interpolation exclusively through
Step
andCycle
selectionsDiagnosticConfig
Fixing of
nan
issue with Structuring 2Performance improvements
dask
for multiprocessing expensive operations locally, with option for distributed computeBEEPDatapath
, tested withsciagraph
memory profiling tool (closes [mat-2532] Improve structuring compute speed and memory usage with dask/modin/swifter #178 ); (done!)Containing different cyclers completely within their classes, fixes Validation and conversion schema can be moved entirely into Datapath children #390
Integration and replacement of
BEEPDatapath
withRun
across codebase including CLIOverhaul of structuring tests for
Run
Pros and Cons vs. previous structuring
BEEPDatapath
, and older*CycleRun
frameworkPros
nan
issue. Everything is actually interpolated.from_file
functionality to load many kinds of cycler datastep_type
and wondering what that actually means)dask.distributed
to declare a local client cluster beforehand and use the code as is - or you can configure it to run on a cluster. I've been able to get the memory usage down to about 1/3 it was withBEEPDatapath
.ProcessedCyclerRun
andBEEPDatapath
objects previously structured and saved to disc.Cons
Step
/Cycle
objects have to be instantiated. Typically ~30s on laptopStep
can cause strange behavior when grouping multiple steps together (i.e., when steps must be grouped together to interpolated based on chg/dchg step labels.)problems:
run.raw.cycles[-1]
does not work, butrun.raw.cycles[1]
will give you the cycle withcycle_index=1
.run.raw.cycles["regular"][3]
won't give you the 3rd regular cycle, it will give you the regular cycle with cycle index == 3. Can be fixed but need to determine what kind of behavior we actually want. For the time being, there is a methodby_raw_index
to retrieve various single objects by their raw indexother notes
step_index
standard column has been renamed tostep_code
step_counter
calculated fromstep_code
Examples: TBA