`Shared<T>` can't be unwrapped or serialized
Closed this issue · 7 comments
I'm interested in getting at the internals of a QLearning in order to serialize just the q_func member. It's the only part relevant to my application after training, and so I'd like to unwrap it from the shared pointer so I can serialize just the wrapped function approximation. However, because the Rc<RefCell<_>> contained within Shared is private, it seems to be impossible to unwrap the contained values in a safe manner---only borrowing is permitted. And, because Shared does not implement Serialize or Deserialize, I can't serialize it all as a unit.
As an alternative, I would suggest making Shared a type alias:
type Shared<T> = Rc<RefCell<T>>;
That would make many useful methods available for working with these values, such as Rc::try_unwrap. After unwrapping a function approximator, I could then serialize it if it implements the proper traits, which many (or all?) in the lfa package do.
Happy to send a pull request if you think this is a good direction to move in.
Another possibility would be to keep Shared as a wrapper (allowing custom trait implementations) but to make the wrapped value public:
pub struct Shared<T>(pub Rc<RefCell<T>>);
I agree, this is not the best solution. To be honest I'm not entirely happy with having to do this kind of referencing in the first place aha!
I think both approaches you have suggested are valid and I'm more than happy for you to contribute. At the moment, I suspect some of this may change in the next major version, so the priority is to ensure compatibility. If you want to put up a PR with your proposed solution I'll take a look through and we can get it merged in.
Note, I also think the former would be better, for all the reasons you have mentioned.
Just sent pull request #72 which is a total of four characters. It just makes the innards of Shared public.
Making Shared a type alias for Rc<RefCell<_>> is also easy in terms of the library itself, but calling code runs into issues due to the loss of the handy trait implementations. I imagine there's some trait magic involving Deref or something that could smooth some of that out a la https://doc.rust-lang.org/book/ch15-02-deref.html
Ah, yes I thought there may have been a reason why it wasn't a type alias to begin with. Your pull request sounds good, so I'll merge it in.
I'll push this as a minor version change as well so you should be able to pull it in from crates.
That's perfect, thanks for a quick turnaround. Only change from your master I'm relying on still is my DerefVec trait (as in #70) but it sounds like you have plans for dealing with that issue in the future.
No problem. And yeah, I'm almost done with my changes to upstream crates which should hopefully make this trait redundant along with a number of other moving parts. I'll ping you on #70 when I've made progress.
Shouldn't be too long, but if it's drags on please feel free to send another comment to refresh.
Tom