-
Notifications
You must be signed in to change notification settings - Fork 526
Update Pendulum-V0 to V1 in reagent tests #673
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
Open
alexnikulkov
wants to merge
648
commits into
facebookresearch:main
Choose a base branch
from
alexnikulkov:export-D38918251
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Update Pendulum-V0 to V1 in reagent tests #673
alexnikulkov
wants to merge
648
commits into
facebookresearch:main
from
alexnikulkov:export-D38918251
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: Prereq for making model managers stateless Reviewed By: czxttkl Differential Revision: D29253385 fbshipit-source-id: 9db747f46a84f26bce079efe8c4394efd3c8adc7
…h#493) Summary: Pull Request resolved: facebookresearch#493 Finally removed normalization data from model manager state Reviewed By: czxttkl Differential Revision: D29253429 fbshipit-source-id: 619b93b473e49b07fe74d0b525d6fc5f30f52550
Summary: Give it to `build_trainer()` directly so that we can remove state in model managers Reviewed By: czxttkl Differential Revision: D29258017 fbshipit-source-id: 39f4a7e8ad9a92499ffeb3c04e2e1c61c10769c0
Summary: Pull Request resolved: facebookresearch#494 - Remove `initialize_trainer()` - Implement `train()` on ModelManager base class; remove all the duplicates - Make `build_serving_module[s]()` takes the trainer module so it can extract whatever nets in the trainer module - `ModelManager.train()` now returns `Tuple[RLTrainingOutput, pl.Trainer]` so that `_lightning_trainer` member can be deleted Reviewed By: czxttkl Differential Revision: D29258016 fbshipit-source-id: 71545dc77c386b532bb48fe4c8ee94c79c20f5c6
Summary: Implement multi-stage trainer module so that multi-stage training looks the same as other training. Internally, the multi-stage trainer forward calls to internal trainers. Reviewed By: czxttkl Differential Revision: D29273266 fbshipit-source-id: b51e91e5670362fc8ed85d9eeb05bd685fc7cbfd
Differential Revision: D29398026 fbshipit-source-id: 76923009da0f6fbc82a9fa8ae96c9417422c2577
Summary: Pull Request resolved: facebookresearch#497 Reviewed By: czxttkl Differential Revision: D29405221 fbshipit-source-id: 3e3524d92fb8d243b7fe62a04830b8f2b80df6ce
Differential Revision: D29458224 fbshipit-source-id: dcef29cd83ee7aecc94100ed579d023072ab581e
Summary: Pull Request resolved: facebookresearch#498 Add some assertions to make sure end users can use algorithms correctly. Reviewed By: bankawas Differential Revision: D29481662 fbshipit-source-id: 0332d990df7d3eca61e1f7bd205136d32f04a7b2
Summary: Pull Request resolved: facebookresearch#499 Remove Seq2SlateDifferentiableRewardTrainer because it's not tested and wouldn't be used. Reviewed By: kittipatv Differential Revision: D29522083 fbshipit-source-id: 9cd7e0d6d1d10c17cc174a54d77a4b37b0f279b7
Summary: Pull Request resolved: facebookresearch#500 Migrate the regular seq2slate to PyTorch Lightning, which includes one model manager `Seq2SlateTransformer` and three trainers `Seq2SlateTrainer`, `Seq2SlateSimulationTrainer` and `Seq2SlateTeacherForcingTrainer`. Manual optimization (https://pytorch-lightning.readthedocs.io/en/latest/common/optimizers.html#manual-optimization) is used to handle the sophisticated usage of optimizers during training. Model manager `Seq2SlatePairwiseAttn` and trainer `Seq2SlatePairwiseAttnTrainer` are not migrated in this diff. But to make them compatible with the changes, the setting of `minibatch_size` is also moved from `trainer_params` to `reader_options`. Reviewed By: czxttkl Differential Revision: D29436608 fbshipit-source-id: 612a1de4923eb7d138fcb6cb4715be6e4d05b424
Summary: AutoDataModule yields dictionary of tensors. Therefore, we need to manually type the input Reviewed By: czxttkl Differential Revision: D29479986 fbshipit-source-id: ab135bb869d8f0eb1fba1813aebf5af6d5ca3401
Differential Revision: D29573192 fbshipit-source-id: 65dc670d1777dd1d6b86c9228a198cd16f504c6e
…h#489) Summary: Pull Request resolved: facebookresearch#489 Reviewed By: czxttkl Differential Revision: D29144000 fbshipit-source-id: b72401ee3bb69f4973c32914a440e571d56241f6
…ebookresearch#502) Summary: Pull Request resolved: facebookresearch#502 Use transformers to learn the return decomposition model. 1) customized attention layers that feed positional encoding to Key & Query but not V. 2) residual connections that learn meaningful embeddings. Reviewed By: czxttkl Differential Revision: D29346526 fbshipit-source-id: c6e642548d4d2b0bcc7f089c08d9144c6f96f8e0
Reviewed By: zertosh Differential Revision: D29656934 fbshipit-source-id: c40bbc8e4512b145050ee47db2c8dc781f3c36e9
…search#501) Summary: Pull Request resolved: facebookresearch#501 Migrate model manager `Seq2SlatePairwiseAttn` and trainer `Seq2SlatePairwiseAttnTrainer` to PyTorch Lightning. This diff marks the completeness of the migration to PyTorch Lightning for the entire reagent codebase. `train_and_evaluate_generic` is removed. Only `train_eval_lightning` from now on! Reviewed By: kittipatv, czxttkl Differential Revision: D29545053 fbshipit-source-id: 71d115c07354b297d3b56d9bfcd13854cd60cb34
Summary: Pull Request resolved: facebookresearch#503 (1) Entropy regularization is added in the CRR to test whether it can help improve the stability of the training or not. (2) Modification in rl_offline_analysis: extract `dqn` manifold path from CRR outputs. Reviewed By: czxttkl Differential Revision: D29469826 fbshipit-source-id: 705ee9069edff9a2b2ff5362d3c4ff464b5a27bd
Summary: There are several modules in the ReAgent library where the logger level is set in the library code thus overriding the level set by the library client resulting in very verbose stdout. This diff removes places in the library where the logger level is set so that the client's setting is always maintained. Reviewed By: bankawas Differential Revision: D29673661 fbshipit-source-id: 8f6db342571d4524768f75d6d6bf4416bad8ad1c
Summary: Delete old style trainer classes Reviewed By: czxttkl Differential Revision: D29700788 fbshipit-source-id: 2f4448d9a7cb8d31d11b25bf35184e1f8c1ce9f6
Differential Revision: D29738340 fbshipit-source-id: 97c83cea89c46c469cdc967cce2ac7ce281c55fc
Summary: Pull Request resolved: facebookresearch#508 Reviewed By: czxttkl Differential Revision: D29805519 fbshipit-source-id: dbcde11f8292eb167a0b7a66384e0d1d723b38e4
Summary: Pull Request resolved: facebookresearch#506 Use `ExtraData.from_dict` Reviewed By: czxttkl Differential Revision: D29768249 fbshipit-source-id: de0056420ab71a79c4f9821cf451328949256037
Summary: Implement data module for Seq2Slate Reviewed By: czxttkl Differential Revision: D29717416 fbshipit-source-id: 424e3c025d73f691c8b0880f853f8d4dca0db584
Summary: Pull Request resolved: facebookresearch#507 Previously the SlateQ trainer only supports SARSA on-policy training. This diff implements a off-policy training approach based on Q-learning. Changes are: 1. Introduced a new `slate_opt_parameters` to specify which slate optimization method to use: top_k, greedy, or exact, based on the SlateQ paper. Currently only the top_k approach is implemented; 2. When choosing the next action, instead of directly using `training_batch.next_action`, we first calculate the Q-value for each next candidate, and rank them by doc value * Q-value. And choose the indices for the top-k items as the next action. Reviewed By: kittipatv Differential Revision: D29660887 fbshipit-source-id: 9b15de4cba41ad5e34f1ca4553f90c53399052c4
Summary: Pull Request resolved: facebookresearch#511 n/a Reviewed By: igfox Differential Revision: D29820857 fbshipit-source-id: 7389785f20e1a503c5eea3221c5ad68ca1f79b31
Summary: Pull Request resolved: facebookresearch#510 Currently QR-DQN is not tested offline. This diff adds an integration test to open_ai_gym_offline and cogwheel. It also corrects an issue with QR-DQN's CPE (the optimizers were in the wrong order) and modifies our model registration to work outside of fblearner flow environments. Reviewed By: kittipatv Differential Revision: D29800557 fbshipit-source-id: ae324c0323a9e644524a228ab296c412923c5336
Summary: Pull Request resolved: facebookresearch#512 - Removing the opt from `manual_backward` call - Pin Lightning version to same version as in fbcode Reviewed By: igfox Differential Revision: D29828482 fbshipit-source-id: 26a52d71362a9a6fd1ea995d854f4a0268d5cce6
Summary: Currently if logged action prob is 0 NaNs can propagate to the actor loss (even with entropy set to 0) and mess up training (f287261291). This diff removes entropy calculation if entropy_coeff <= 0 and raises an error if entropy calculation is on while a logged action has probability 0. Reviewed By: czxttkl Differential Revision: D29861744 fbshipit-source-id: 2fae30e7108145139851d0767d7bbe18f6dd388a
Summary: Black/whitelist are in the process of being removed from all FB code (https://fb.workplace.com/groups/e/permalink/3320810064641820/). This diff replaces all instances of black/whitelist with block/allowlist in the reagent codebase. Reviewed By: kittipatv Differential Revision: D29881070 fbshipit-source-id: 3d2e63eff5f4371f994ba4ae37586e3ef33c2fb7
Summary: Pull Request resolved: facebookresearch#647 X-link: meta-pytorch/torchrec#447 Add a static function to concat a list of KJTs Reviewed By: dstaay-fb Differential Revision: D36944002 fbshipit-source-id: 1b6865f60dcea91ee250b69360e4606184ffad53
Summary: Made LinUCB neural-based to enable distributed training. Differential Revision: D37009962 fbshipit-source-id: 4bb3e68ea60a264d26e13e4ce19832bca67a2c7e
Differential Revision: D37353746 fbshipit-source-id: b0dc7a8b59f4c6a1e39daa67fbae1e519488b6ef
Differential Revision: D37371791 fbshipit-source-id: 8247af28ab27242782ab8d317e7d9763121bcc74
Summary: Pull Request resolved: facebookresearch#649 fix world model reporter so that we can read losses per epoch make the logic needed to perform at the end of a train/test/validation epoch more explicit in reagent_lightning_module Differential Revision: D37305377 fbshipit-source-id: 2204cfe94269cfba839b72c77bfea341ab63637d
Summary: We want to make the EB/EBC scriptable by default w/o the need of running torch.fx first. Not able to script EB/EBC modules by default (especially when needed for inference) is very non-intuitive and inconvenient. In the same time we don't plan to make ShardedEB/EBC scriptable. 1. Do not use property, use methods instead. This is consistent w/ KJT. 1. _embedding_bag_configs has a complex type List[EmbeddingBagConfig] which does not script. Use List[str] to store features instead. Pull Request resolved: facebookresearch#648 X-link: meta-pytorch/torchrec#467 Reviewed By: colin2328 Differential Revision: D37389962 fbshipit-source-id: 5ce079a946b9458ee63658cae2fd731cfc1c7958
Summary: Pull Request resolved: facebookresearch#650 In the base LightningModule class we define an optional `logger` property which may be initialized to None, while in DQNtrainer._log_dqn method we try to access the `logger` object without checking first if it was initialized. The issue surfaced when trying to run unit tests analogous to those in `test_qrdqn`. This commit adds a check whether the `logger` is initialized prior to attempting to use it. Interestingly, the analogous QRDQNTrainer class implementation does not use the `logger` property for logging, perhaps it's redundant? Reviewed By: czxttkl Differential Revision: D37529027 fbshipit-source-id: 5fe81cf715ee9f759b937290f1184d1c67e5325f
Summary: Pull Request resolved: facebookresearch#651 Adds the test_dqn.py with a set of unit tests for DQNTrainer class, mirroring those in test_qrdqn.py Reviewed By: czxttkl Differential Revision: D37536537 fbshipit-source-id: 60cef76adb62c54e66b3fda39596c1cf0ad20555
…#658) Summary: Pull Request resolved: facebookresearch#658 1. update machine images for some cpu-only tests 2. we have to switch back to use torchrec nightly (instead of stable) because the torchrec cpu stable version has caused some error in circle ci tests. See for example: https://app.circleci.com/pipelines/github/facebookresearch/ReAgent/2437/workflows/1c7658b7-1197-425d-9fe3-166876d492ac/jobs/23829 Reviewed By: speedystream Differential Revision: D38101590 fbshipit-source-id: d72f2cf0d204598ef648e0969522e9801029eca4
Summary: Pull Request resolved: facebookresearch#659 two things will help avoid openssl errors when installing python 3.8: upgrade pyenv add a retry logic for installing python 3.8. Reviewed By: speedystream Differential Revision: D38123668 fbshipit-source-id: 7e527a0caf2d302a81306b7f2005e92ce19a6f5e
Summary: To avoid test failures caused by importing torchrec, i finally decide the following import rules: For gpu machines, import torchrec (gpu, stable version) For cpu machines, import torchrec-nightly-cpu Reviewed By: speedystream Differential Revision: D38185498 fbshipit-source-id: 7988695f827cfd04d53f6d63630ac843eb6c23ee
Summary: Pull Request resolved: facebookresearch#657 1. add docstrings 2. test if models are torch.jit.trac-able Reviewed By: dkorenkevych Differential Revision: D38067071 fbshipit-source-id: 7863e0e1f3f618ee7fe46c6fa076fec1dd6fd48a
Summary: Pull Request resolved: facebookresearch#653 Add test_dqn_base.py file with unit tests for the methods in DQNTrainerBaseLightning class. Reviewed By: czxttkl Differential Revision: D37673366 fbshipit-source-id: 43482dde9be06a0df1e8dd3bb16e92d508bc8a13
…ebookresearch#654) Summary: Pull Request resolved: facebookresearch#654 Add docstrings to DQNTrainer and DQNTrainerBaseLightning classes and their methods. Reviewed By: czxttkl Differential Revision: D37875900 fbshipit-source-id: 52e9947f1c84f099bedb79a696de94a05c631f5c
Summary: Pull Request resolved: facebookresearch#663 This is the first part of the diff. I have moved the model in to reagent. Moreover, I have made some refactoring. Reviewed By: czxttkl Differential Revision: D38011818 fbshipit-source-id: cb76646ed0e3149887180cbe642b1035afaace9b
Differential Revision: D38447983 fbshipit-source-id: 03d4384d075a57bfbd9a76c23730307fc5255c90
Summary: Pull Request resolved: facebookresearch#665 we need to unfold embeddings from different sparse features Differential Revision: D38556778 fbshipit-source-id: 8eb646105991c0307d981bb3198c48e850cededa
) Summary: Pull Request resolved: facebookresearch#666 ReAgent trainer inputs usually inherit from `TensorDataClass`. In GPU training, trainer inputs are moved to cuda automatically by pytorch lightining strategies. We need to make sure `TensorDataClass.to(torch.device("cuda"))` can move all fields to cuda. Before this change, KeyedJaggedTensor in a TensorDataClass cannot be moved to cuda properly. Reviewed By: alexnikulkov Differential Revision: D38685842 fbshipit-source-id: 96c35a8171e3249a9cbcdd561f38d64078b3e9a6
Summary: Pull Request resolved: facebookresearch#664 Implement Reagent DeepRepresentLinUCB. It is a multiple layer regression model that output UCB score. The first N layers are trainable by torch optimizer(). The last layer is the traditional LinUCB, and it is not updated by optimizer, but still will be updated by matrix computations. {F760388037} Reviewed By: alexnikulkov Differential Revision: D38268001 fbshipit-source-id: d739e6af0cfecd891681d1e736d3191547441c92
Summary: Pull Request resolved: facebookresearch#668 Previously if we do `super__init__(automatic_optimization=automatic_optimization)` (which we better do), the CI complains for some unknown reason. This quick fix use `**kwargs` to include `automatic_optimization` . Reviewed By: alexnikulkov Differential Revision: D38771283 fbshipit-source-id: 89062959ee0de2d2bca9caa961c146d16708a633
…gument in pytorch (facebookresearch#670) Summary: Pull Request resolved: facebookresearch#670 See title Reviewed By: czxttkl Differential Revision: D38839005 fbshipit-source-id: f720555dc9caf840c9354eca4ef8d86101ae7fe3
Summary: Pull Request resolved: facebookresearch#669 Add `DeepRepresentLinUCBTrainerParameters`. Reviewed By: alexnikulkov Differential Revision: D38813086 fbshipit-source-id: e1cbe01ed100b9a8328fe6cd63e9672e1df269f1
Summary: Pull Request resolved: facebookresearch#671 CircleCI tests started failing. Example: https://app.circleci.com/pipelines/github/facebookresearch/ReAgent/2490/workflows/efbceb68-9a01-4889-8d82-3d4167af4643/jobs/24780 Root cause is a significant jump in protobuf version from 3.19.4 to 4.21.5. This was caused by an an updated dependency on protobuf in grcpio-tools. I'm adding an upper limit on grcpio-tools version to prevent this error Reviewed By: BerenLuthien, czxttkl Differential Revision: D38870120 fbshipit-source-id: 10d2e619aa10c2b03272b8305681792feefd06f4
Summary: Pull Request resolved: facebookresearch#672 The test was failing in CircleCI: https://app.circleci.com/pipelines/github/facebookresearch/ReAgent/2491/workflows/e153e4b9-795d-4c30-a3ff-6e4411b7dbf2/jobs/24796 I removed unnecessary parts which were breaking the test Reviewed By: czxttkl Differential Revision: D38874439 fbshipit-source-id: 74fe760014b5152d8a59f676beb4c8fa96e15798
Summary: Code design doc: - First part of https://docs.google.com/document/d/1eiyJpLSiSDufPc4h2nIRrHh4yllBtZ9DEsFaHMtl6GM/edit# Main difference from existing LinUCB model in the codebase: - Input is not a batch of data, but a List[batch] of data. Each element of the input is the corresponding batch for an arm. - Similarly: -- covariance matrix A and inv_A is (num_arms, d, d) dimension -- model coef: (num_arms, d) Reviewed By: alexnikulkov Differential Revision: D38374089 fbshipit-source-id: c60b990d8cb7d75ab334ceb24beebf4f5b78fc8a
Summary: This should unblock a failing autodatamodule test (f366492605) Gym documentation says V1 behaves same as V0: {F763157287} Differential Revision: D38918251 fbshipit-source-id: 48ef6557799c23192841b51c8cae303897d0914a
This pull request was exported from Phabricator. Differential Revision: D38918251 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This should unblock a failing autodatamodule test (f366492605)
Gym documentation says V1 behaves same as V0: {F763157287}
Differential Revision: D38918251