Skip to content

Cleanup train project #410

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.

Sign up for GitHub

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

Jump to bottom
Merged
wbenoit26 merged 5 commits into from
Oct 23, 2025
Merged

Cleanup train project #410

wbenoit26 merged 5 commits into from
Oct 23, 2025

Conversation

@wbenoit26
Copy link
Contributor

@wbenoit26 wbenoit26 commented Oct 20, 2025

Addresses a couple of edge cases and cleans up some of the training code. @EthanMarx This is based on #409, so once that's merged in this can follow.

@wbenoit26 wbenoit26 force-pushed the cleanup-train-project branch from a3752f7 to cb8f567 Compare October 21, 2025 19:53
@wbenoit26
Copy link
Contributor Author

wbenoit26 commented Oct 23, 2025

@EthanMarx Bump on this

EthanMarx
EthanMarx reviewed Oct 23, 2025
View reviewed changes
projects/train/train/data/base.py
return int(self.hparams.fduration * self.hparams.sample_rate)

@property
def left_pad_size(self) -> int:
Copy link
Contributor

@EthanMarx EthanMarx Oct 23, 2025

Choose a reason for hiding this comment

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

So the user still specifies left_pad which means the spacing from the whitened kernel right?

Copy link
Contributor Author

@wbenoit26 wbenoit26 Oct 23, 2025

Choose a reason for hiding this comment

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

Yeah, that's correct.

EthanMarx
EthanMarx approved these changes Oct 23, 2025
View reviewed changes
Copy link
Contributor

@EthanMarx EthanMarx left a comment

Choose a reason for hiding this comment

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

This is great - tons of nice fixes and generalizations we've needed.

A lot of this was inspired by Bhavyas issues using BNS waveforms right?

@wbenoit26
Copy link
Contributor Author

wbenoit26 commented Oct 23, 2025

Yeah, the fixes in slice_waveforms and build_val_batches came from edge cases he ran into. Really, there also needs to be better handling in how we slice the waveforms in build_val_batches as well. But I'm starting to lean towards just having better error catching, rather than try to take care of any possible combination of kernel length, waveform duration, and coalescence time.

@wbenoit26 wbenoit26 merged commit 56f6a1e into ML4GW:main Oct 23, 2025
5 checks passed
@wbenoit26 wbenoit26 deleted the cleanup-train-project branch October 23, 2025 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@EthanMarx EthanMarx EthanMarx approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wbenoit26 @EthanMarx