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

Merged Box Example Distributions #66

Merged
merged 9 commits into from
Jun 21, 2023
Merged

Conversation

josephdviviano
Copy link
Collaborator

Sorry for the nasty commit history on this one.

This is an example code that shows how to handle torchgfn in the case that actions can be sampled from two distributions, depending on the state. In this example, when t=0, we sample from one distribution, and when t>0, we sample from a second distribution. Therefore, each batch element of the States tensor must sample from one of two distributions.

DistributionWrapper exposes a .sample() method which sequentially samples from QuarterDisk and QuarterCircleWithExit. These two output spaces are zero-padded so that they are cross-compatible.

BoxPFNeuralNet similarly computes both the S_t=0 parameters, and the S=t>0 parameters, for all batch elements, and then replaces the S=t>0 outputs with the S_t=0 parameters where nessicary.

Note that the number of parameters used for S_t=0 and S=t>0 can differ. We therefore also mask the BoxPFNeuralNet outputs additionally to account for when the smaller number of parameters is utilized.

TODO (In a Follow Up PR):

  • Simplify the code. I don't think we need the BoxPFEstimator and BoxPFNeuralNet to be two distinct classes. It would be easier if all the logical existed in the BoxPFEstimator. But we can debate this.
  • Ensure the Tabular case works as intended.
  • Put the tests in a proper place. But where?

Extras

  • Upgraded pyright to 1.1.314.
  • Alphabetical sorting and comments in pyproject.toml.
  • Identities torch.pi /2 and 2 / torch.pi expressed as globals in box_utils.py.
  • Bugfixes in original tests.
  • TODOs throughout.

@josephdviviano josephdviviano self-assigned this Jun 20, 2023
@josephdviviano josephdviviano changed the title Merged distributions Merged Box Example Distributions Jun 20, 2023
@saleml
Copy link
Collaborator

saleml commented Jun 21, 2023

Thanks for the great PR ! I'll merge now, and move the TODOs to a new issue.

@saleml saleml merged commit 707dd66 into generic-gfn Jun 21, 2023
@josephdviviano josephdviviano deleted the merged_distributions branch August 2, 2023 01:25
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.

2 participants