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

Possible codebase restructure #221

Open
younik opened this issue Nov 19, 2024 · 3 comments
Open

Possible codebase restructure #221

younik opened this issue Nov 19, 2024 · 3 comments

Comments

@younik
Copy link
Collaborator

younik commented Nov 19, 2024

I propose here an alternative codebase organization, with:

gfn.core: implementing the interfaces of States, Action, GFlowNet base file, base Env class, Trajectories, Transitions, and ReplayBuffer.
gfn.discrete: depends on gfn.code interfaces, and implements the DiscreteStates, DiscreteEnv and everything that is specifically to Discrete, like FMGFlowNet.
gfn.graph Similarly to before, everything that is related to Graph.

Motivation:
Currently, we are mixing different state-type implementations in the same file creating unnecessary dependencies and reducing modularity. Same for environments.
For example, for Graph, I need torch_geometric, but a user may want to use the library only for discrete env and not install torch_geometric.

Dividing in submodules increases modularity and makes it easier to scale the codebase to different use cases

@saleml
Copy link
Collaborator

saleml commented Nov 19, 2024

I like the overall idea. I'd love to see a tree of the new structure!

Where would classes used for the continuous examples belong in this new structure ?

@younik
Copy link
Collaborator Author

younik commented Nov 19, 2024

Where would classes used for the continuous examples belong in this new structure ?

Good point; box and line directly sublcass Env, and doesn't need special States. We may leave them in gnf.core or create a gfn.continuous

I am also wondering if we will introduce new Action types in the future that can be mixed with other State types. This will make the submodules confusing.

@josephdviviano
Copy link
Collaborator

josephdviviano commented Nov 19, 2024 via email

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

No branches or pull requests

3 participants