GFNOrg / torchgfn

A modular, easy to extend GFlowNet library
https://torchgfn.readthedocs.io/en/latest/
Other
233 stars 30 forks source link

Preprocessor refactoring #220

Open younik opened 2 days ago

younik commented 2 days ago

I believe preprocessors should not be part of the Env class, and they should kept separate.

1) These lines in env examples don't scale well: https://github.com/GFNOrg/torchgfn/blob/6f132a869902584c06b7f8054e76dda696d78c94/src/gfn/gym/hypergrid.py#L58-L72

2) It seems they are not used inside the env class, but they are just passed to GFNModule, which is a better location.

https://github.com/GFNOrg/torchgfn/blob/6f132a869902584c06b7f8054e76dda696d78c94/testing/test_gflownet.py#L40-L46

I believe they should be instantiated by the user and eventually passed to GFNModule.

josephdviviano commented 2 days ago

Yes this makes sense to me. The GFNModule would be able to handle raw and pre-processes states using whatever preprocessor is passed to it.

Joseph Viviano @josephdviviano https://twitter.com/josephdviviano viviano.ca

On Sun, Nov 17, 2024 at 10:31 AM Omar Younis @.***> wrote:

I believe preprocessors should not be part of the Env class, and they should kept separate.

1.

These lines in env examples don't scale well:

https://github.com/GFNOrg/torchgfn/blob/6f132a869902584c06b7f8054e76dda696d78c94/src/gfn/gym/hypergrid.py#L58-L72 2.

It seems they are not used inside the env class, but they are just passed to GFNModule, which is a better location.

https://github.com/GFNOrg/torchgfn/blob/6f132a869902584c06b7f8054e76dda696d78c94/testing/test_gflownet.py#L40-L46

I believe they should be instantiated by the user and eventually passed to GFNModule.

— Reply to this email directly, view it on GitHub https://github.com/GFNOrg/torchgfn/issues/220, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7TL2X27CXAQSGOXTIC5HT2BCZFNAVCNFSM6AAAAABR6BK5QSVHI2DSMVQWIX3LMV43ASLTON2WKOZSGY3DMMBQHAZDENQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>