Nysa/cleaned up ipad#21
Open
nysapk wants to merge 7 commits into
Open
Conversation
mgagvani
reviewed
Apr 29, 2026
| class LitModel(pl.LightningModule): | ||
| def __init__(self, model: nn.Module, lr: float, lr_vision: float | None = None, rfs_weight: float = 0.0): | ||
| def __init__( | ||
| self, |
Owner
There was a problem hiding this comment.
Instead of making these args in the LitModel, create an iPadConfig class that contains all of these, in the same file as the iPad class definition, so that it can just reference the class.
mgagvani
reviewed
Apr 29, 2026
| pred_scores = outputs.get("scores", None) | ||
| pred_traj_flat = outputs.get("trajectory_flat", None) | ||
| query_for_score = outputs.get("query_for_score", None) | ||
| pred_scores = None |
Owner
There was a problem hiding this comment.
probably want a way to handle this without breaking what was already there
mgagvani
reviewed
Apr 29, 2026
Owner
There was a problem hiding this comment.
plz don't delete stuff randomly
mgagvani
reviewed
Apr 29, 2026
Owner
There was a problem hiding this comment.
don't think we need this to be in the main branch, i realize you were debugging but it makes it messier
mgagvani
reviewed
Apr 29, 2026
Owner
There was a problem hiding this comment.
no reason there should be any diff on this file? maybe you need to merge in the current main
mgagvani
reviewed
Apr 29, 2026
Owner
There was a problem hiding this comment.
please do not commit sbatch scripts
mgagvani
reviewed
Apr 29, 2026
Owner
There was a problem hiding this comment.
i think you need to merge in main
mgagvani
reviewed
Apr 29, 2026
mgagvani
reviewed
Apr 29, 2026
Owner
There was a problem hiding this comment.
ok we can definitely remove this 😭
mgagvani
reviewed
Apr 29, 2026
Owner
There was a problem hiding this comment.
don't think this necessarily needs to make it to main either
mgagvani
reviewed
Apr 29, 2026
| parser.add_argument('--batch_size', type=int, default=16, help='Batch size for training') | ||
| parser.add_argument('--lr', type=float, default=1e-4, help='Learning rate') | ||
| parser.add_argument('--max_epochs', type=int, default=10, help='Number of epochs to train') | ||
| parser.add_argument('--num_proposals', type=int, default=16, help='Number of proposals (proposal model only)') |
Owner
There was a problem hiding this comment.
again i dont want to add a ton of args to the train script so instead add them to a config dataclass
mgagvani
reviewed
Apr 29, 2026
Owner
There was a problem hiding this comment.
its deleted so probably need to merge in main
mgagvani
reviewed
Apr 29, 2026
mgagvani
requested changes
Apr 29, 2026
…d code - Remove debug_callbacks.py, debug_viz.py (debugging artifacts not for main) - Remove CHANGES.md, environment.yml (reviewer requested removal) - Remove run_train_proposal.sbatch and run_train_proposal_login_gpu.sh (no sbatch/scripts in main) - Restore monocular.py, viz.py, and models/transfuser to match main (no reason for diff) - Restore pred_traj_flat, query_for_score variables and dict extraction in base_model.py - Add back type annotations and descriptive ValueError messages in base_model.py Made-with: Cursor
…riority - Add IPadConfig dataclass in proposal_planner.py bundling all iPad-specific loss/scorer hyperparameters (matches reviewer request to keep config in same file as the iPad model class). - LitModel now accepts a single ipad_config argument instead of 17 individual kwargs. Defaults are preserved via the dataclass defaults; hparams are populated from the config so all internal getattr lookups still work. - train.py constructs an IPadConfig only for the proposal model. - Restore original IMAGES-first batch key priority in _shared_step so existing IMAGES batches keep their previous code path (per reviewer feedback). - Drop --debug / --debug_log_every flags and references to deleted debug_callbacks / debug_viz modules from train.py. Made-with: Cursor
create_vocab.py:
- Trivial trailing-comma comment difference, resolved.
base_model.py:
- LitModel.__init__: combine ipad_config hparam approach with main's
has_nvjpeg flag and model.cfg introspection for wandb logging.
- decode_batch_jpeg: take main's improved signature (device param,
cam_idxs_used selection, nvjpeg fallback on AMD GPU).
- Output dict handling: keep PR's raw_output + proposal_list approach.
- Score loss: unify iPad BCE/CE scorer (PR) with legacy ScorerModel
mse/reinforce/margin path (main) via model_cfg_loss_type hparam.
- Total loss: keep PR's already-computed formula, drop main's duplicate.
- log_dict: combine PR's refinement oracle ADEs with main's scorer_metrics
update + batch_size/sync_dist args.
train.py:
- Remove duplicate import os.
- Keep PR's ProposalPlanner/IPadConfig imports + iPad argparse flags.
- Add HomogeneousConcatBatchSampler from main for multi-dataset support.
- Add --dataset flag from main (waymo/nuscenes/all).
- Keep PR's --model_type / backbone selection; capture model name for
dynamic run timestamp.
- Logger: keep --no_wandb toggle; use main's dynamic timestamp and ddp
strategy; add use_distributed_sampler + torch.set_float32_matmul_precision.
- Callbacks: use main's inline ModelCheckpoint style.
Co-authored-by: Cursor <cursoragent@cursor.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
cleaned up ipad code a bit