Conversation
Dagster development
Dagster development
Formatting
Add placeholder unit test for likelihood module
- Reformat long function calls and asset specs for clarity - Use consistent string quoting and indentation - Move assert error messages to multi-line format for readability - Add properties to SingleEventLikelihood for duration and detector names - Refactor likelihood classes for modularity and clarity
Add abstract _likelihood method to SingleEventLikelihood and refactor subclasses to implement core likelihood logic in _likelihood. Move parameter updates and fixed values to evaluate methods for clarity.
Expand unit tests for likelihood classes and add fixtures
Fix HeterodynedTransientLikelihoodFD initialization
…-in-single_eventlikelihoodpy 208 replacing keyword arguments in single event likelihood
|
Caution Review failedThe pull request is closed. WalkthroughThis update is a comprehensive refactor and expansion of the codebase, introducing new container build files, workflows, and a major reorganization of the core modules for gravitational wave parameter estimation. It adds new abstractions for priors, transforms, likelihoods, data handling, detectors, and run management, while removing legacy single_event modules. Extensive new tests, documentation, and Dagster pipelines are included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RunManager
participant RunDefinition
participant Jim
participant Likelihood
participant Prior
participant Transforms
User->>RunManager: Instantiate with RunDefinition or config file
RunManager->>RunDefinition: Load or initialize run config
RunManager->>Jim: Initialize with Likelihood, Prior, Transforms, and sampler params
User->>RunManager: sample()
RunManager->>Jim: sample()
Jim->>Prior: sample_initial_condition()
Jim->>Likelihood: evaluate_posterior()
Jim->>Transforms: apply sample/likelihood transforms
Jim->>Jim: Run sampler (training/production)
Jim-->>RunManager: Return samples, log-prob, loss, diagnostics
RunManager-->>User: Provide results, plots, summary
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes This is a major refactor and expansion involving new core abstractions, removal of legacy code, extensive new features, and broad test coverage. It touches nearly every aspect of the codebase and will require careful, multi-disciplinary review. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation - BetaCodeRabbit's unit test generation is now available in Beta! Automatically generate comprehensive unit tests for your code changes, ensuring better test coverage and catching edge cases you might miss. Our AI analyzes your code structure and creates tests that follow best practices and your project's testing patterns. Learn more here, or just try it under ✨ Finishing Touches. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
This pull request includes many development changes and bug fixes since the last revamp of the prior system.
The major updates in this PR involves the now more standardized
runinterface, which should be used for production-related task. Another major feature updates is the incorporation of orchestration tools likeDagster, in order to provide an interface for fully automated deployment. Note that the orchestration example and tools are not directly within thejimpackage, but we provide examples of deploying such system. The run interface is relatively stable going forward, while the orchestration interface is still in development mode.Another major update in this PR is the restructuring of the single event likelihood interface, specially standardizing the class hierarchy of the likelihood classes. Different marginalization schemes are now in their own class, such that parameters are standardized as class variables instead of run-time dictionary values.
Once we hit production level stable, we should start keeping a change log and define the release schedule of jim.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style
Tests
Chores
Revert