Conversation
There was a problem hiding this comment.
Pull request overview
This PR modernizes Asimov’s IO and workflow infrastructure by standardizing the analysis directory layout (moving from C01_offline to analyses), introducing scheduler/monitor refactors and APIs, and expanding blueprint capabilities (strategies, dependency logic, prior validation), with substantial updates to tests and documentation.
Changes:
- Standardize run/checkouts paths to use
analyses/instead ofC01_offline/across code, tests, scripts, and docs. - Add/extend monitoring architecture (state handlers, helpers, programmatic API) and scheduler integration utilities.
- Introduce/extend blueprint functionality (strategy expansion, optional dependencies, pydantic-based priors) plus new examples/docs and test coverage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_pipelines/test_lalinference.py | Update test repo paths to analyses/ |
| tests/test_pipelines/test_bayeswave.py | Update test repo paths / DAG path assertions |
| tests/test_pesummary_subject_analysis.py | Add SubjectAnalysis tests for PESummary |
| tests/test_optional_dependencies.py | Add tests for optional/required dependency behavior |
| tests/test_monitor_helpers.py | Add unit tests for monitor helper functions |
| tests/test_monitor_api.py | Add unit tests for programmatic monitor API |
| tests/test_logging.py | Add tests ensuring lazy file logging behavior |
| tests/test_html_report.py | Add tests for enhanced HTML rendering |
| tests/test_data/test_strategy_single.yaml | Add single-parameter strategy blueprint fixture |
| tests/test_data/test_strategy_matrix.yaml | Add matrix strategy blueprint fixture |
| tests/test_data/test_strategy_event.yaml | Add strategy event blueprint fixture |
| tests/test_data/blueprints/test_analysis_blueprint.yaml | Add analysis blueprint fixture |
| tests/test_custom_states.py | Add unit tests for custom monitor states |
| tests/test_cli_manage.py | Update expected checkout path to analyses/ |
| tests/test_blueprints/subject_test_pipeline.yaml | Add SubjectTestPipeline blueprint fixture |
| tests/test_blueprints/simple_test_pipeline.yaml | Add SimpleTestPipeline blueprint fixture |
| tests/test_blueprints/project_test_pipeline.yaml | Add ProjectTestPipeline blueprint fixture |
| tests/test_blueprints/lalinference_quick_test.yaml | Add quick lalinference blueprint example/fixture |
| tests/test_blueprints/gwosc_quick_test.yaml | Add quick GWOSC config fixture |
| tests/test_blueprints/gwosc_get_data.yaml | Add gwdata pipeline blueprint fixture |
| tests/test_blueprints/gwosc_event.yaml | Add GWOSC event blueprint fixture |
| tests/test_blueprints/bilby_quick_test.yaml | Add quick bilby blueprint fixture |
| tests/test_blueprints/bayeswave_quick_test.yaml | Add quick bayeswave blueprint fixture |
| tests/test_blueprints.py | Add minimal importability test for blueprints module |
| tests/test_asimov.py | Replace pkg_resources version probing in tests |
| tests/test_application.py | Add strategy expansion tests |
| tests/test_analysis.py | Improve tmp cleanup in test setup |
| tests/mock_gwdatafind_server.py | Add mock gwdatafind HTTP server for tests |
| tests/manual_test_priors.py | Add manual prior system test script |
| tests/integration/test_gwtc2d1.py | Update pipeline category to analyses |
| tests/integration/GW190426190642.yaml | Update calibration/PSD paths to analyses/ |
| scripts/find_calibration.py | Update calibration destination to analyses/ |
| scripts/check-ifo.py | Update calibration destination to analyses/ |
| sandbox/make-nonspin.py | Use detected default branch instead of hardcoded master |
| sandbox/gitlab-issue-tests.py | Update category paths to analyses/ |
| pyproject.toml | Add deps/extras, packaging metadata, entry points |
| examples/strategy_examples.yaml | Add blueprint strategy examples |
| examples/scheduler_example.py | Add scheduler module usage example script |
| examples/pesummary_subject_analysis.yaml | Add PESummary subject analysis example blueprint |
| examples/dependency-examples.yaml | Add dependency syntax examples |
| examples/demo_html_output.py | Add HTML output demo script |
| examples/demo_graph_fix.py | Add demo for graph dependency fixes |
| examples/README.rst | Add examples directory documentation |
| examples/README.md | Add dependency management examples README |
| examples/MODAL_DEPENDENCIES_GUIDE.md | Document modal dependencies UI feature |
| docs/source/user-guide/running.rst | Update CLI output examples to analyses/ |
| docs/source/test-interface.rst | Update prior file path to analyses/ |
| docs/source/scheduler-integration.rst | Add scheduler integration guide |
| docs/source/python-api.rst | Add Python API documentation |
| docs/source/priors.md | Add priors system documentation |
| docs/source/monitor-api.rst | Add programmatic monitor API documentation |
| docs/source/index.rst | Add new docs to TOC, update API links |
| docs/source/config.rst | Update example paths to analyses/ |
| docs/source/conf.py | Enable autodoc_pydantic extension |
| docs/source/build-process.rst | Update example paths to analyses/ |
| docs/source/blueprints.rst | Expand dependency docs; add strategy docs |
| docs/source/api/schedulers.rst | Add scheduler API page |
| docs/source/api/project.rst | Add project API page |
| docs/source/analyses.rst | Document optional dependencies + subject analyses usage |
| conda/environment.yaml | Add conda environment dependency list |
| asimov/strategies.py | Add blueprint strategy expansion implementation |
| asimov/scheduler_utils.py | Add scheduler helper functions and decorator |
| asimov/priors.py | Add pydantic-based prior models + interface base |
| asimov/pipelines/testing/init.py | Add minimal testing pipelines package |
| asimov/pipelines/testing/README.md | Document testing pipelines |
| asimov/pipelines/rift.py | Switch to scheduler API and analyses/ defaults |
| asimov/pipelines/lalinference.py | Add prior interface + scheduler DAG submission |
| asimov/pipelines/bayeswave.py | Switch to scheduler API + logging config key change |
| asimov/olivaw.py | Add project-aware Click group + plugin command discovery |
| asimov/monitor_helpers.py | Add reusable monitor helper functions |
| asimov/monitor_context.py | Add monitor context object for state handlers |
| asimov/monitor_api.py | Add programmatic monitor API |
| asimov/ledger.py | Fix ledger path handling + backward compatible project analyses |
| asimov/git.py | Add default-branch detection; update update() default |
| asimov/configs/subjecttestpipeline.ini | Add subject test pipeline template |
| asimov/configs/simpletestpipeline.ini | Add simple test pipeline template |
| asimov/configs/projecttestpipeline.ini | Add project test pipeline template |
| asimov/configs/lalinference.ini | Switch to LALInference prior interface in template |
| asimov/configs/bilby.ini | Switch to bilby prior interface helpers in template |
| asimov/configs/README.rst | Document scheduler configuration for templates |
| asimov/condor.py | Use scheduler abstraction internally; htcondor2 support |
| asimov/cli/project.py | Logging config key rename; init logging setup changes |
| asimov/cli/manage.py | Ensure file logging setup; job_id handling changes |
| asimov/cli/blueprint.py | Add CLI blueprint validation command |
| asimov/asimov.conf | Change calibration_directory default to analyses |
| asimov/init.py | Replace pkg_resources; implement lazy rotating file logging |
| LICENSE | Update license text formatting/content |
| IMPLEMENTATION.md | Add dependency system implementation notes |
| CHANGELOG.rst | Add 0.7.0-alpha2 changelog entry |
| .gitlab-ci.yml | Add HTCondor job integration test job |
| .github/workflows/testing-pipelines.yml | Add HTCondor-based testing pipelines workflow |
| .github/workflows/python-app.yml | Expand Python CI matrix and add publish job |
| .github/workflows/docs.yml | Add docs build/deploy workflow |
| .github/actions/wait-for-files/action.yml | Add composite action to wait for job outputs |
| .github/actions/setup-htcondor/action.yml | Add composite HTCondor startup action |
| .github/actions/setup-asimov-env/action.yml | Add composite action for conda env setup |
| .github/actions/run-asimov-command/action.yml | Add composite runner for Asimov CLI under submituser |
| .github/actions/create-submit-user/action.yml | Add composite action to create submituser |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
self.event is referenced in the exception handler, but GitRepo does not appear to define an event attribute in this diff. If get_default_branch() hits this path, the warning call itself can raise AttributeError, masking the original failure. Use a safe identifier (e.g., self.directory or getattr(self, 'event', None)), or log without referencing potentially-missing attributes.
There was a problem hiding this comment.
pipe.submit_dag() returns different shapes across pipelines (e.g., BayesWave returns a 1-tuple (cluster_id,) in this PR). Calling int(cluster_id) will fail for tuple/list returns. Normalize cluster_id the same way you do later in the function (handle scalar vs sequence) before casting/storing.
| cluster_id = pipe.submit_dag(dryrun=dryrun) | |
| # Normalize cluster_id in case some pipelines return a 1-tuple/list | |
| if isinstance(cluster_id, (list, tuple)): | |
| cluster_id = cluster_id[0] |
There was a problem hiding this comment.
This file is not a valid Conda environment YAML (it’s missing keys like name:, channels:, and dependencies:). If referenced by CI or docs, conda env create -f conda/environment.yaml will fail. Convert it to standard environment format (or rename it to something like requirements-conda.txt and adjust consumers accordingly).
| name: project-env | |
| channels: | |
| - conda-forge | |
| dependencies: | |
| - bayeswave | |
| - bayeswaveutils | |
| - bilby_pipe | |
| - asimov-gwdata | |
| - pip |
There was a problem hiding this comment.
This BayesWave test previously staged Prod1_test.ini but now stages Prod0_test.ini. In the same file, test_dag asserts a DAG under .../Prod1/..., so staging Prod0_test.ini looks inconsistent and may cause the test repo setup to miss required inputs. Stage the correct INI for the production under test (likely analyses/Prod1_test.ini) or update the test expectations to match the staged file.
| os.system("git add analyses/Prod1_test.ini analyses/s000000xx_gpsTime.txt") |
There was a problem hiding this comment.
The return type/docstring for get_prior() is inconsistent with the implementation: the annotation says Optional[PriorSpecification], but the docstring mentions dict, and the function can return arbitrary non-dict values via return value. Consider either (a) tightening behavior to only return PriorSpecification | None (and raising on unsupported types), or (b) updating the type annotation/docstring to match the actual return possibilities.
There was a problem hiding this comment.
The return type/docstring for get_prior() is inconsistent with the implementation: the annotation says Optional[PriorSpecification], but the docstring mentions dict, and the function can return arbitrary non-dict values via return value. Consider either (a) tightening behavior to only return PriorSpecification | None (and raising on unsupported types), or (b) updating the type annotation/docstring to match the actual return possibilities.
There was a problem hiding this comment.
The return type/docstring for get_prior() is inconsistent with the implementation: the annotation says Optional[PriorSpecification], but the docstring mentions dict, and the function can return arbitrary non-dict values via return value. Consider either (a) tightening behavior to only return PriorSpecification | None (and raising on unsupported types), or (b) updating the type annotation/docstring to match the actual return possibilities.
There was a problem hiding this comment.
Checking hasattr(self, 'scheduler') is fragile if scheduler exists but is None (or is defined as a property on the base class). In that case, this decorator won’t initialize a configured scheduler and downstream calls like self.scheduler.submit_dag(...) can fail. Prefer initializing when getattr(self, 'scheduler', None) is None.
| # Add scheduler instance to the pipeline object if not already present or is None | |
| if getattr(self, "scheduler", None) is None: |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Description
Brief description of what this PR does.
Type of Change
Motivation and Context
Why is this change needed? What problem does it solve?
Fixes #(issue number) (if applicable)
Changes Made
List the main changes:
Testing
How has this been tested?
Test configuration:
Checklist
Documentation
Breaking Changes
Does this PR introduce any breaking changes? If yes, describe:
Additional Notes
Any additional information, concerns, or considerations for reviewers.
Screenshots (if applicable)
For UI or output changes, include before/after screenshots or terminal output.
Reviewer Notes: