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

add support for including Graphs as States of GFlowNet #183

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ashdtu
Copy link
Collaborator

@ashdtu ashdtu commented Aug 23, 2024

Describe your changes

Type: Feature
Add support for including Graphs as States of GFlowNet
The Graph structure is represented via the Data class of Torch Geometric. The GraphStates object is represented via the Batch class which encapsulates batching of Data objects and their efficient indexing. As opposed to the existing States object which requires appending dummy states for batching different length Trajectories together, we seek to support Trajectories by representing it as nested Batch object.

The existing implementation of Trajectory supports the following indexing dimensions: (Num time steps, Num trajectories, State Size). The nested Batch of Batch object representing state Trajectories would naturally indexing of the form (Num trajectories, Num timesteps, State size), and would need to implement logic for flipping the indexing dimensions internally in _getitem_() and _setitem_().

Issue Number

#153

Testing

  • Unit tests for helper functions of GraphState class: initialization, appending, batching etc.
  • Compatibility check with Trajectories, Transition class
  • Unit testing with FlowMatching : accessing GraphStates directly in loss function
  • Unit testing with TrajectoryBalance: accessing GraphStates via trajectory class

@josephdviviano josephdviviano self-assigned this Sep 22, 2024
@LicoriceLin
Copy link

Hi, is there any further plan on implementing this feature? What's the bugs so far? Is there anything I could help out?

And if I just want to hack it, can I simply copy the GraphStates class as a temporary solution?

@josephdviviano
Copy link
Collaborator

josephdviviano commented Nov 1, 2024

Hi @LicoriceLin yes there is a plan and it's now a top priority. A few of us will be looking at adding this feature starting next week and we would love your help and or feedback on the implementation to ensure we're able to support your needs. CC @younik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants