GFNOrg / torchgfn

GFlowNet library
https://torchgfn.readthedocs.io/en/latest/
Other
209 stars 26 forks source link

Minimally fix typing issues #141

Closed marpaia closed 10 months ago

marpaia commented 11 months ago

This PR is an alteration to the fix shipped in #139. While we should still iterate on and decide on whether or not this generic type parameter adds more value than it detracts, this change is a minimal diff which aims to at least gets master in a better state.

PFBasedGFlowNet

Originally we had:

class PFBasedGFlowNet(GFlowNet)

Which was causing an error because PFBasedGFlowNet didn't define the generic parameter for the sample type.

This was updated to:

class PFBasedGFlowNet(GFlowNet, Generic[TrainingSampleType]):

But it should be instead:

class PFBasedGFlowNet(GFlowNet[TrainingSampleType]):

This is basically saying that PFBasedGFlowNet is a kind of GFlowNet where the TrainingSampleType is not yet known.

TrajectoryBasedGFlowNet

TrajectoryBasedGFlowNet didn't need to be changed so this change reverts that part of #139 so that the signature is back to:

class TrajectoryBasedGFlowNet(PFBasedGFlowNet[Trajectories]):

This implies that TrajectoryBasedGFlowNet is a kind of PFBasedGFlowNet which uses Trajectories as the sample type.

josephdviviano commented 10 months ago

Thanks very much for the fix of the fix!