Skip to content

BNS MDC bug fixes#423

Merged
wbenoit26 merged 11 commits intoML4GW:bns-o3-mdcfrom
wbenoit26:bns-bug-fixes
Feb 3, 2026
Merged

BNS MDC bug fixes#423
wbenoit26 merged 11 commits intoML4GW:bns-o3-mdcfrom
wbenoit26:bns-bug-fixes

Conversation

@wbenoit26
Copy link
Contributor

@deepchatterjeeligo @bhgupta20 This should be enough to get things running. The biggest issue was an API change in the Decimator object, which we should have made a minor release for, rather than a point release. AMPLFI uses v0.7.8, while the preprocessor was written expecting at least v0.7.10. As a workaround, I've pinned the version in online to v0.7.8 and modified the preprocessor accordingly.

Otherwise, there were just some minor corrections.

@bhgupta20
Copy link
Contributor

bhgupta20 commented Feb 3, 2026

@wbenoit26 thank you for the fixes. I have a question, why did you change this to 12 per day?

Also, I think split is not an argument for the forward function, but instantiated in the beginning, I think that is why the test is failing?

@wbenoit26
Copy link
Contributor Author

I've reverting the ml4gw version back to when split was an argument in the forward function.

I changed the threshold to a FAR of 12 per day because that's the standard threshold for uploading an event during the MDC. We just had the default set incorrectly.

Copy link
Contributor

@deepchatterjeeligo deepchatterjeeligo left a comment

Choose a reason for hiding this comment

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

@wbenoit26 I am wondering if it is a better idea to put the requires-python >= 3.10 and then updating the lock files as a separate MR to main, and rebasing this branch. Those changes seem atomic enough. You can create a new branch from main and cherry-pick the pyproject.toml changes.

@wbenoit26
Copy link
Contributor Author

@deepchatterjeeligo I was thinking the same thing. Once I get the environment issues sorted out, I'll make the changes to main.

@wbenoit26
Copy link
Contributor Author

@deepchatterjeeligo There's actually enough differences between what I'd want to do on main and what I need to do here that I think it makes more sense to do things separately. Eventually, we can update AMPLFI to use to same Decimator API and train a new model, etc. at which point the environments will be back in sync.

@wbenoit26 wbenoit26 merged commit 5aa758a into ML4GW:bns-o3-mdc Feb 3, 2026
9 checks passed
@wbenoit26 wbenoit26 deleted the bns-bug-fixes branch February 3, 2026 19:03
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.

3 participants