-
Notifications
You must be signed in to change notification settings - Fork 32
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
Simplify API: Cleans-up some API details around Model and Module #158
Conversation
parameters, collections = collections | ||
elif "parameters" in collections: | ||
parameters = collections.pop("parameters") | ||
else: |
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.
@alexander-g added the cases above so its compatible with the previous output of get_default_parameters
(a dict with a "parameters" key) and the new version (a tuple (params, collections)). Feel free to clean this up if you update the weights.
Codecov Report
@@ Coverage Diff @@
## master #158 +/- ##
==========================================
- Coverage 84.90% 84.85% -0.05%
==========================================
Files 131 131
Lines 6815 6840 +25
==========================================
+ Hits 5786 5804 +18
- Misses 1029 1036 +7
Continue to review full report at Codecov.
|
Changes
elegy.States
now implementsMapping
and__getattr__
so you can add arbitrary keys but still usestates.{field}
to access values. By not enforcing any particular set of fieldsStates
can be more flexibly used with the low-level API, however, the user must know what the are the standard names (rng
,net_params
,net_states
,metrics_states
, etc) and their behavior, this will be properly documented.States
(rng
,net_params
,net_states
, ...) from the Dependency Injection lists, now the users must requeststates
and access the field of interest (e.g.states.rng
), this is both simpler and more explicit.elegy.Module.init
andelegy.Module.apply
now return(y_pred, parameters, collections)
whereparameters = collections.pop("parameters", None)
.apply
now expectsparameters
andcollections
as arguments.base_step
to the low-level API, defines the initial set ofelegy.States
.PredStep
, it is now only(y_pred, states)
.Uninitialized
type.summary_step
and movessummary
toModelCore
.