Skip to content
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

Develop var len obs feature #4909

Merged
merged 52 commits into from
Feb 12, 2021
Merged

Develop var len obs feature #4909

merged 52 commits into from
Feb 12, 2021

Conversation

vincentpierre
Copy link
Contributor

@vincentpierre vincentpierre commented Feb 4, 2021

Proposed change(s)

Adding support for variable length observations.
Adding a new environment to test it : Sorter
TODO :

  • Need to modify the changelog
  • Wait for barracuda 1.3.1 for faster rank 3 matrix multiplications on CPU

Types of change(s)

  • [ ] Bug fix
  • New feature
    - [ ] Code refactor
  • [ ] Breaking change
  • Documentation update
  • [ ] Other (please describe)

Checklist

  • Added tests that prove my fix is effective or that my feature works
  • Updated the changelog (if applicable)
  • Updated the documentation (if applicable)
  • [ ] Updated the migration guide (if applicable)

Other comments

@vincentpierre vincentpierre marked this pull request as draft February 4, 2021 00:33
@vincentpierre vincentpierre self-assigned this Feb 4, 2021
{
private int m_MaxNumObs;
private int m_ObsSize;
float[] m_ObservationBuffer;
int m_CurrentNumObservables;
static DimensionProperty[] m_DimensionProperties = new DimensionProperty[]{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: s_DimensionProperties

the `BufferSensor` will be able to collect.

To add an entity's observations to the `BufferSensor`, you need to call the
`BufferSensor.AppendObservation()` (or `BufferSensorComponent.AppendObservation()`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a little misleading since you can't access the BufferSensor directly (at the moment)

Copy link
Contributor

@Hunter-Unity Hunter-Unity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pulled the scene. found one issue with buffer sensor obsv size that vince fixed. confirmed his fix resolved the issue. looks good now.

@vincentpierre vincentpierre merged commit d583283 into master Feb 12, 2021
@delete-merged-branch delete-merged-branch bot deleted the develop-var-len-obs-feature branch February 12, 2021 18:55
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants