-
Notifications
You must be signed in to change notification settings - Fork 3
Introduce decorator to describe 1d spaces and regime ids. Fixes #192. #208
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
Conversation
…ecutive integers in describing 1d spaces. Fixes #192.
|
Sorry for the mess... |
timmens
left a comment
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.
Very nice. I have mainly two comments:
- I think the naming style is very different compared to other parts of lcm. Maybe rather something like
@lcm.space_dimension. Or we actually also differentiate between state and action here (just like in the type annotations) and do@lcm.state_space_dimensionand@lcm.action_space_dimension. - I am not a fan of removing the explicit enumeration. I think a big advantage of the explicit enumeration was that users could debug more easily. Now users need to count, which is of course error prone, if they need to check which dimension of their Markov array corresponds to which category, or if they debug lcm.
|
|
|
Great. How about This is explicitly similar to pandas' notion of categoricals since it is precisely the same idea. A natural way of passing transition matrices would be a DataFrame with a MultiIndex of (typically) Age and the current states and actions / future states, each of which have CategoricalDtypes corresponding to the attributes of the dataclasses decorated with |
I'd prefer that. We won't have too many outcomes, typically. And it is near-trivial to look at the actual |
timmens
left a comment
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.
I like this!
This gets rid of having to specify consecutive integers in describing 1d spaces manually.
Simple enough, but I am not perfectly sure about the naming yet.