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

[BugFix, Feature] GNN position_key and velocity_key not in observation_spec #125

Merged
merged 9 commits into from
Sep 3, 2024

Conversation

matteobettini
Copy link
Collaborator

@matteobettini matteobettini commented Sep 3, 2024

fixes #124

Previously

GNN position_key and velocity_key needed to be part of the observation_spec

This meant that they could not be used in a squence model where the GNN is not first as they would be accidentally consumed by other models

Now

Now these keys don't need to be in observation_spec.

In fact, it is better that they are not to solve the problem above.

They can be put in the info.

We still support the case of them being in observation_spec for bc-compatibility

The catch

Since now we do not know the shape of these keys (as they are not in the spec) we need to provide it in the model configuration. This is because GNNs need to know the shape of edge_features at init time.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 3, 2024
@matteobettini matteobettini marked this pull request as ready for review September 3, 2024 16:58
@matteobettini matteobettini changed the title [BugFix] GNN position_key and velocity_key [BugFix, Feature] GNN position_key and velocity_key not in observation_spec Sep 3, 2024
@matteobettini matteobettini merged commit aef8d40 into main Sep 3, 2024
13 checks passed
@matteobettini matteobettini deleted the gnn_pos branch September 3, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using GNNs for models
2 participants