SideChannel helpers #3596
SideChannel helpers #3596
Conversation
| return str; | ||
| } | ||
|
|
||
| public IList<float> ReadFloatArray() |
There was a problem hiding this comment.
Why not a float[]?
Not sure we need this method to be honest.
There was a problem hiding this comment.
I added it on the python side in response to @awjuliani's comment on the doc.
float[] implements the IList<float> interface; I figured it was better to be a bit more generic in case we ever wanted to change the underlying implementation.
com.unity.ml-agents/Runtime/SideChannels/FloatPropertiesChannel.cs
Outdated
Show resolved
Hide resolved
| public void SetRawBytes(byte[] data) | ||
| { | ||
| m_Stream.Seek(0, SeekOrigin.Begin); | ||
| m_Stream.Write(data, 0, data.Length); |
There was a problem hiding this comment.
If the stream was created with an array that was larger than the one passed it, won't the stream be corrupted after this?
There was a problem hiding this comment.
Good catch. I think m_Stream.SetLength(0) should fix this (and setting capacity as an optimization too).
There was a problem hiding this comment.
Added a unit test and confirmed fixed.
There was a problem hiding this comment.
don't you always want the capacity to be data.Length?
There was a problem hiding this comment.
I didn't want to shrink capacity if we didn't need to.
| } | ||
| } | ||
|
|
||
| /// <summary> |
There was a problem hiding this comment.
Put in a separate file
There was a problem hiding this comment.
Yeah, makes sense, wasn't sure where to draw the line.
| } | ||
| } | ||
|
|
||
| /// <summary> |
There was a problem hiding this comment.
Put in separate file
Proposed change(s)
Utilities for incoming and outgoing sidechannel messages. Still requires the user to make sure fields are written in a consistent order, but means that they never have to deal with bytes or struct pack/unpack directly.
This is still a work in progress; if you think this will be useful, I can add more methods and add some actual docstrings/update the docs. Can also add similar abstractions on the C# side if they'd be helpful.
Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)
Types of change(s)
Checklist
Other comments