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

Refactor tasks #98

Merged
merged 50 commits into from
Aug 1, 2023
Merged

Refactor tasks #98

merged 50 commits into from
Aug 1, 2023

Conversation

bengioe
Copy link
Collaborator

@bengioe bengioe commented Jul 6, 2023

This PR refactors tasks into reusable components that will make it easier to define new tasks and methods.

The PR is pretty big, but a lot of it is moving code around (or deleting repeated code):

  • Creates a notion of Conditional, implements TemperatureConditional, MultiObjectiveWeightedPreferences, FocusRegionConditional
  • Separates the commonalities between seh_frag and seh_frag_moo into a more generic StandardOnlineTrainer that is meant to be easily subclassed for new tasks.
  • Adds some implementation notes and comments
  • Adds a validate_batch routine that's useful for debugging, e.g. new environments and datasets

Also fixes some bugs:

  • Makes valid_offline_ratio a flag and sets it explicitly in tasks where it wasn't properly set
  • first_graph_idx was incorrectly calculated in SubTB (affected logging of logZ values)
  • QM9Dataset was returning the wrong shape for its rewards
  • Adds a allow_5_valence_nitrogen flag to MolBuildingEnvContext, this is needed in some cases, see tasks/qm9.py.
  • Adds an explicit stop_mask to MolBuildingEnvContext.graph_to_Data
  • Fixes incorrect default objective name in seh_frag_moo
  • Fixes the default configurations in the tasks' main that hadn't been updated
  • Fixes a number of routines where focus_cond was assumed to exist (but we can now turn it off).

@bengioe bengioe marked this pull request as ready for review July 20, 2023 19:01
@bengioe bengioe requested review from hadim and maclandrol July 20, 2023 19:01
Copy link

@hadim hadim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not much to say!

Just a simple curiosity question below.

src/gflownet/tasks/config.py Show resolved Hide resolved
Copy link

@maclandrol maclandrol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments.

src/gflownet/data/sampling_iterator.py Outdated Show resolved Hide resolved
src/gflownet/envs/mol_building_env.py Show resolved Hide resolved
src/gflownet/models/graph_transformer.py Show resolved Hide resolved
src/gflownet/tasks/config.py Show resolved Hide resolved
src/gflownet/tasks/config.py Show resolved Hide resolved
src/gflownet/tasks/qm9/qm9.py Outdated Show resolved Hide resolved
src/gflownet/tasks/qm9/qm9.py Outdated Show resolved Hide resolved
src/gflownet/trainer.py Show resolved Hide resolved
@bengioe bengioe merged commit 152b18f into trunk Aug 1, 2023
5 checks passed
@bengioe bengioe deleted the refactor_tasks branch August 1, 2023 16:02
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.

4 participants