distance grid export#135
Open
oshaughn wants to merge 35 commits into
Open
Conversation
Owner
oshaughn
commented
May 26, 2026
- pseudo_pipe: enable extrinsic export
- demo/rift/ : demo file
Add RIFT.precision and route all extended-precision dtype use through it.
* RIFT.precision (new): RiftFloat resolves to numpy.longdouble whenever
the platform's long double has itemsize > 8 (e.g. Linux x86_64), and
otherwise falls back to numpy.float64. Also exports
RIFT_FLOAT_HIGH_PRECISION and RIFT_FLOAT_NAME. Eliminates the
import-time AttributeError when numpy.float128 is absent (macOS arm64,
Windows MSVC, non-x86 Linux, future numpy 2.x platforms).
* Integrator package: replace every numpy.float128 / np.float128 in
mcsampler.py, mcsamplerEnsemble.py, mcsamplerGPU.py,
mcsamplerAdaptiveVolume.py, mcsamplerNFlow.py, mcsamplerPortfolio.py,
statutils.py
with RiftFloat. The dtype-equality guards in mcsamplerGPU and
mcsamplerNFlow ("if weights_alt.dtype == numpy.float128: cast to
float64") degrade gracefully when RiftFloat == float64 (the
conditional astype becomes a no-op).
* likelihood/factored_likelihood.py and
interpolators/BayesianLeastSquares.py: same RiftFloat swap, so they
also import cleanly on platforms without np.float128.
* CI (.github/workflows/ci.yml): add rift_O4d_gmm_gpu to the trigger
branches; expand the install matrix to 3.9-3.13; convert
import-check and test-run into a two-lane matrix:
- legacy : python 3.9 + numpy==1.24.4 (historical green build)
- modern : python 3.12 + numpy>=2.0,<3.0 (forward-looking gate)
Numpy is pinned after requirements.txt, so the unpinned 'numpy' line
in requirements.txt is preserved. Test-log artifacts are named
per-lane so failures from each can be uploaded independently.
No behavioral change on the existing legacy CI lane: RiftFloat is
numpy.longdouble there, which is the exact 16-byte type previously
spelled numpy.float128. The modern CI lane is the new gate.
Implement a reusable distance-grid export helper for ILE, thread the export flag through pseudo_pipe, and add focused reconstruction tests. Add a zero-spin fake-data demo that builds a DAG with distance-grid export enabled, plus a small lalsimutils XML compatibility fix for current LAL bindings.
… 4.4.0 causes breaking changes)
Owner
Author
|
Not happy with AI changes to the algorithm -- lots of testing needed. |
Provide a root pixi workspace that defaults local development to SWIG <4.4.0 while also defining a SWIG >=4.4.0 comparison environment. Add GitLab CI jobs for both pixi environments so deployment stability can be checked across the hidden SWIG binding change.
* RIFT/misc/distance_grid.py: build_distance_grid now divides out the distance sampling prior so the exported lnL is L_pure(d) = integral L(d,Omega) pi_Omega dOmega. New column ln_prior_d_sampling carries the per-bin sampling-prior factor so default reconstruction reproduces log_res exactly, while reconstruct_marginal_lnL(grid, ln_prior_d=...) re-marginalizes against any prior of choice. * integrate_likelihood_extrinsic_batchmode: handle mcsamplerEnsemble/GMM _rvs columns (raw integrand/joint_prior/joint_s_prior, not log_*); drop zero-weight samples cleanly instead of raising "missing type". Pass sampler.prior_pdf["distance"] values per-sample. * test/test_distance_grid.py: cover the pure-likelihood property (different priors yield correctly different marginals) and the round trip. * demo/rift/add_distance_grids/validate_distance_grid.py: new stress harness quantifies n_eff vs integral/shape error. * pixi.toml: pin lalsuite==7.25, lalmetaio<=4.0.5 to dodge the SWIG-4.4 cross-module SwigPyObject/LIGOTimeGPS regression (issue oshaughn#136). Verified end-to-end ILE run produces .dgrid whose reconstruction matches log_res to machine precision. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ILE batchmode now optionally emits a .dslice file per intrinsic point containing K independent extrinsic-marginalized likelihoods at K distance slice centers (quantile centers of the posterior in d). The estimator is importance-reweighting of the main run's Omega samples at each slice distance, re-using the cached likelihood machinery -- no waveform or PSD regeneration, no extra worker spin-up. With K~=10 the artifact stays within the user's <~10x .composite size budget. * RIFT/misc/distance_slices.py: importance_reweight_slices, quantile_slice_centers, table builder/loader, and reconstruct_marginal_lnL that takes an optional custom distance prior. Schema (DISTANCE_SLICE_FIELDS) deliberately mirrors .composite for downstream CIP integration. * integrate_likelihood_extrinsic_batchmode: new --export-distance-slices K and --distance-slice-method flags; threaded into analyze_event after the main integration. Reuses sampler._rvs and like_to_integrate. Emits a runtime warning when GMM + low main n_eff, since B2-reweight silently biases in that regime. * demo/rift/add_distance_grids/validate_distance_slices.py: synthetic stress harness with a known closed-form marginal and an adjustable d-Omega coupling. Confirms B2-reweight matches truth to <0.1 nat over a wide coupling range when main n_eff is healthy. * demo/rift/add_distance_grids/PLAN_B_DESIGN.md: design notes covering the math, the GMM-vs-AV finding, the (recommended) non-destructive workflow integration plan, and the deferred B2-fresh cross-check path. End-to-end check on the fake-data demo (AV sampler, main n_eff ~6): B2 marginal reconstructs the main log_res within sigmaL; per-slice n_eff 7-28; .dslice 10 rows x 19 columns per event. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reweight alone breaks in the tails: Omega samples drawn during the main
run have no support at distances far from the posterior peak, so the
slice estimator silently biases or returns garbage there. Switch to a
hybrid scheme where core slices stay reweight (cheap, accurate inside
the posterior) and wing slices are fresh Omega-only AdaptiveVolume
integrations at the pinned distance (correct, expensive only on the few
points we need them).
* RIFT/misc/distance_slices.py:
- fresh_sample_slices builds a fresh AV sampler over Omega only, clones
the main sampler's per-param (pdf, prior, llim, rlim) config, wraps
like_to_integrate to pin distance and defensively clip Omega values
inside [llim, rlim] (avoids arccos NaN at boundary).
- pick_wing_centers places K_wing centers log-uniformly in
[d_min, d_core_lo] union [d_core_hi, d_max], evenly split.
- is_uninformative detects a flat-in-d core so wings are skipped on
events where the distance posterior carries no information.
- sigma_lnL conversion for fresh slices: AV returns log(rel_var) +
2*log_int; we report sqrt(rel_var) so the column is on the same
scale as the reweight branch and the main run's sigmaL_main.
* integrate_likelihood_extrinsic_batchmode: split --export-distance-slices
K into --n-distance-slice-core (reweight) + --n-distance-slice-wing
(fresh). Default 60/40 split. New flags --distance-slice-wing-nmax,
--distance-slice-wing-neff, --distance-slice-skip-threshold.
Per-row method column (reweight=0, fresh=1) marks which estimator
produced each slice.
* PLAN_B_DESIGN.md: documents the architecture and the empirical wing
reach on the demo event (~30 nats below peak with sigmaL ~0.1-0.2,
well past the ~7-nat-target for 10^{-3} prior weight outside).
End-to-end on the fake-data demo (AV, --n-distance-slice-core 6 --n-distance-slice-wing 4):
main log_res 59.09 +/- 0.30, n_eff 4.4
core slices: lnL 60.8-62.3 (peak-lnL 0-1.4 nat), sigmaL 0.19-0.55
wing slices at 23/376/613 Mpc: lnL 59/51/34 (peak-lnL 3/11/29 nat),
sigmaL 0.10-0.22, neff 9-24
far wing at 5 Mpc: lnL -100 (signal off), correctly flagged low neff
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two follow-ups left for the next session, called out as breadcrumbs
rather than implemented now:
1. Skip threshold should be an absolute lnL scale.
lnL is already a likelihood ratio with absolute meaning; the
current relative spread test will misfire on high-SNR events
whose distance posterior happens to be flat.
2. Wing centers from a parabolic-in-1/dist fit of the core, solved
for the 1/dist values where lnL drops by ~7 nats from peak
(probability outside ~10^{-3}). Marginalized-lnL caveat (the
inclination-distance ridge can extend further toward small d
than a simple parabola predicts) documented inline.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements the two PLAN_B_DESIGN breadcrumbs: 1. is_uninformative now applies an absolute lnL detectability cut (peak core lnL < threshold) instead of a relative max-min spread test. lnL is a likelihood ratio vs noise, so this correctly skips undetected low-SNR events while keeping high-SNR events with a flat distance profile. ILE skip message and --distance-slice-skip-threshold help updated. 2. pick_wing_centers fits the core (lnL, 1/d) points to a parabola in 1/d (fit_lnL_parabola_in_inv_d) and spans each wing from the core edge out to where the model drops --distance-slice-wing-delta-lnL nats below peak (default 7), via _parabolic_wing_bounds. Bounds are clamped to the sampler's distance support; degenerate fits fall back to the original log-uniform full-range placement. New ILE flag --distance-slice-wing-delta-lnL threads the target. Adds a regression test (test_wing_placement_and_skip) to validate_distance_slices.py; verified end-to-end on the fake-data demo (AV sampler): wings concentrate near the core instead of the prior edges, and reconstruct_marginal_lnL matches log_res within sigmaL. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ine builder Threads per-distance likelihood export from util_RIFT_pseudo_pipe.py through create_event_parameter_pipeline_* onto the ILE extrinsic stage (ILE_extr.sub), with an end-to-end pipeline-build test/demo. CEPP (Basic/Alternate/BasicMultiApprox): - New flags --last-iteration-export-distance-slices K plus -n-core/-n-wing/-wing-delta-lnL/-skip-threshold passthroughs. When set, the extrinsic stage gets --export-distance-slices K (+ the tunables + --internal-use-lnL) and --distance-marginalization is stripped, mirroring the existing grid export. - AlternateIteration and BasicMultiApproxIteration previously lacked the grid flag entirely; added both the grid and slice args + the ile_args_extr handling so the subdags/multi-approx CEPP variants accept the flags pseudo_pipe now routes to them. util_RIFT_pseudo_pipe.py: - New --export-distance-slices K (+ tunables), sibling to --export-marginal-distance-grid. - When either export is requested: force ILE lnL mode, disable distance marginalization (sane auto-config instead of erroring), and warn if --add-extrinsic is absent (the export is emitted there). - Fix: the --last-iteration-export-* flags are pipeline-builder flags, not ILE flags. They were being appended to args_ile.txt (the ILE argument string), where they would have been passed to the ILE executable and rejected. Move them to the CEPP command; keep only the ILE-side hygiene (lnL mode, no distance marginalization) in args_ile. Make the three create_event_parameter_pipeline_* scripts executable (100644 -> 100755), matching their sibling bin scripts: pseudo_pipe invokes them by bare name, so editable/source/pixi installs need +x. Validation: - New demo MonteCarloMarginalizeCode/Code/demo/pipeline (Makefile + README): builds baseline/grid/slices pipelines from the reference ini and asserts the flags land in ILE_extr.sub (and not in the intrinsic ILE.sub), with no distance marginalization. All pass. - Expanded .travis/test-build.sh with the same grid + slice build assertions (run in GitLab and GitHub CI). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… done in PLAN_B_DESIGN Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ic stage Fix the per-distance export threading so distance marginalization is kept for the intrinsic ILE iterations (a large speedup) and removed ONLY on the final extrinsic stage that emits the per-distance output. Previously util_RIFT_pseudo_pipe.py set opts.internal_marginalize_distance = False and stripped --distance-marginalization from args_ile.txt, which disabled it for every ILE job in every iteration. Now pseudo_pipe only forces ILE lnL mode globally (clean lnL-scaled helper args) and leaves distance marginalization in place; create_event_parameter_pipeline_* already strips the standalone --distance-marginalization flag from the ILE_extr argument string, so the disable is confined to the export stage. Make util_InitMargTable executable (100644 -> 100755): the helper invokes it at build time to generate the distance-marginalization lookup table, which is now needed again because the intrinsic stage keeps distance marginalization. Validation updated to prove the last-stage-only invariant: demo/pipeline and .travis/test-build.sh now assert the standalone --distance-marginalization flag is present on the intrinsic ILE.sub / args_ile.txt but absent from ILE_extr.sub (matching the standalone flag via a trailing space, so the harmless leftover --distance-marginalization-lookup-table arg is not counted). All three demo targets pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…n is disabled only at the extrinsic stage Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…validation demo
Adds the consolidation step the previous threading work was missing, plus a
self-contained zero-spin IMRPhenomD demo that runs the whole chain (pipeline
build -> ILE_extr -> consolidate -> posterior) end-to-end without condor.
Pipeline:
- New util_ConsolidateDistanceGrids.py: concatenates per-event .dgrid/.dslice
files (header-checked) into a single net intrinsic+distance table.
- New write_consolidate_distance_grids_sub in RIFT.misc.dag_utils_generic:
mirrors write_cat_sub (extrinsic posterior samples) so the consolidation
plugs into the same post-extrinsic part of the DAG.
- create_event_parameter_pipeline_BasicIteration: when the last-iteration
per-distance export is on, emit consolidate_dgrid.sub /
consolidate_dslice.sub gated on the corresponding flag, build a DAG node,
and chain it as a child of every ILE_extr job (.dgrid / .dslice come
directly off ILE with no convert/resample step, so the consolidation node
parents the ILE_extr nodes, not the cat_node downstream).
Output: all_dgrid.dat / all_dslice.dat at the run root.
Demo + validation:
- demo/pipeline/Makefile: updated grid/slices assertions to require the
consolidation sub-file and DAG references.
- demo/pipeline/zero_spin_phenomD/: new end-to-end test. Uses the
.travis/ILE-GPU-Paper zero-noise BBH fake data, IMRPhenomD with
--assume-nospin, AV sampler. Steps:
build -> util_RIFT_pseudo_pipe.py constructs the pipeline and
asserts the extrinsic stage carries grid export + AV +
IMRPhenomD + lnL mode, distance marg only off at the
extrinsic stage, consolidate_dgrid in the DAG.
run-extr -> bypass condor; invoke ILE_extr directly on N_EVENTS
grid rows -> per-event .dgrid files.
consolidate -> util_ConsolidateDistanceGrids.py -> all_dgrid.dat.
posterior -> util_ConstructEOSPosterior.py with --parameter m1 -m2 -dist
reconstructs the joint (intrinsic+distance) posterior.
Whole chain in ~45 s on a laptop core. Ships a minimal zero_spin_phenomD.ini
whose [rift-pseudo-pipe] section deliberately omits approx /
ile-sampler-method so the CLI overrides win (the ini section parser
otherwise overrides the command line).
Drive-by fix needed for the demo:
- util_ConstructEOSPosterior.py had CRLF line endings, breaking its
/usr/bin/env shebang ("python\r" not found). Converted to LF.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… in PLAN_B_DESIGN Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…--pipeline-builder hot-swap
The --use-subdags path (create_event_parameter_pipeline_AlternateIteration)
was broken for normal runs by a chain of issues, each masking the next:
- cip_args_list parsing crashed on the 'Z'/'G' prefixes emitted by
util_RIFT_pseudo_pipe.py (ValueError: invalid literal for int() ... 'Z').
Ported BasicIteration's tolerant prefix parsing.
- argparse rejected 8 options the helper passes (extrinsic samples-per-ile,
time-resampling, batched-convert, ile-request-disk, cip-explode-jobs-dag/-last,
n-iterations-subdag-max). Added them with BasicIteration's signatures; wired
the ones with a clean home, documented the rest as accepted-but-not-acted-on.
- completed the half-built extrinsic batched/time-resampling convert path
(batchConvertExtr_job was referenced but never defined) by porting
BasicIteration's 3-branch convert setup + node construction.
- fixed undefined unify_node_list used to attach SCRIPT POST composite checks.
AlternateIteration now builds a complete DAG end-to-end from a standard
pseudo_pipe invocation.
Apply the same int('Z') tolerant-parsing fix to BasicMultiApproxIteration,
which had the identical crash.
Thread AlternateIteration into util_RIFT_pseudo_pipe.py as a first-class
drop-in via a new --pipeline-builder {BasicIteration,AlternateIteration}
selector that overrides the implicit --use-subdags routing, enabling
side-by-side A/B testing of the two builders from an otherwise identical
command line. Warns if an explicit choice contradicts an AMR/subdag
requirement.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The mcsamplerEnsemble GPU port (and MonteCarloEnsemble / gaussian_mixture_model) had never been exercised with cupy installed and crashed in GPU mode. Validated end-to-end with .travis/test-integrate.sh on a Kepler/sm_30 card using a cupy 10.6 + cudatoolkit 10.2 environment (last CUDA supporting sm_30). mcsamplerEnsemble: * evaluate()/calc_pdf(): bridge the host integrand/prior -- convert samples to CPU, call the user function, push the result back to the active backend. * replace cupy-incompatible rot90([list]) with an order-preserving reshape. * build dim-group / bounds dict keys with host ints (range/np.arange); the self.xpy.arange variant produced unhashable 0-d cupy arrays on GPU. * return scalars and store _rvs on the host so downstream numpy code works. gaussian_mixture_model / MonteCarloEnsemble: * portable _xpy_logsumexp (cupyx.scipy.special.logsumexp is absent in the cupy CUDA 10.2 build needed for sm_30). * _near_psd: use Hermitian eigh/eigvalsh on GPU (cupy.linalg has no eig/eigvals); the matrices are symmetric. numpy path unchanged. * gpu_logpdf: cupy.linalg has no LinAlgError and cholesky returns NaN rather than raising; catch numpy's error type and treat a NaN factor as failure. All integrators: import cupyx.scipy.special explicitly (not auto-loaded by import cupyx in older cupy). mcsamplerGPU / mcsamplerAdaptiveVolume (so the full test passes in GPU mode): * use instance converters / self.xpy instead of module-level GPU converters, which were contaminating these otherwise-CPU samplers with cupy arrays; * bridge their CPU prior/integrand; ones_like to follow the data backend. No behavioral change without cupy: all GPU branches are guarded by cupy_ok. Note: the AC sampler's --as-test check is statistically flaky (no seed) on both CPU and GPU and can randomly fail; this is pre-existing and unrelated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… dep canary Three related pieces for container CI and flexible multi-architecture deployment: Feature C (core): container family manifest. New RIFT/misc/container_manifest.py parses a YAML manifest (advertising a family of images + GPU capability ranges) and builds HTCondor expressions. Wired into write_ILE_sub_simple and write_CIP_sub (dag_utils_generic.py, import-guarded): when SINGULARITY_RIFT_IMAGE points at a .yaml/.yml manifest, MY.SingularityImage becomes an expression-valued ifThenElse over the matched machine's GPU capability (default GPUs_Capability), selecting the right image per machine. Only the matched image is fetched (CVMFS images referenced in place / lazy-fetched; osdf images selectively transferred via a comma-free $$() ternary token), never the whole family. A require_gpus capability floor is &&-composed with any user RIFT_REQUIRE_GPUS. Plain .sif / osdf:// values keep byte-identical legacy behavior. Vanilla universe throughout. Feature B: multi-target build. New containers/ dir -- rift_container.def.in template + build_family.sh (build matrix; first entry keeps the current production base for broad compatibility), shared requirements-container.txt (single source of truth), example rift_container_family.yaml, and README. Feature A: CI dependency-resolution canary. New non-blocking container-dep-canary and container-swig-canary jobs in ci.yml, plus a weekly schedule, to catch upstream breakage (e.g. swig>=4.4.0, issue oshaughn#136) before a container rebuild. Tests: MonteCarloMarginalizeCode/Code/test/test_container_manifest.py (13 tests: parser, expression builders, integration via write_ILE_sub_simple condor_cmds, all-cvmfs, backward-compat). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Validated on a real HTCondor pool + GPU (cap-3.0 machine): GPUs_Capability /
Capability attribute names, require_gpus floor matching+exclusion, $$()
match-time image selection, and tolerance of the empty-result ("") case for a
mixed CVMFS/osdf manifest. Mixed manifests are safe; uniform retrieval is not
required. Only the OSG/GWMS pilot evaluation of the expression-valued
MY.SingularityImage remains to be smoke-tested on a real glidein.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
New docs/source/containers.rst (wired into the toctree, cross-linked from osg.rst): how to point SINGULARITY_RIFT_IMAGE at a YAML container-family manifest, the manifest schema, what the pipeline generates (expression-valued MY.SingularityImage, selective $$() transfer, require_gpus floor), the GPU attribute names + verification command, building a family with containers/build_family.sh, and the CI dependency canary. Renders under the existing Sphinx 'docs' CI job. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
build_family.sh rendered defs ran `pip3 install -r containers/requirements-container.txt` from inside the cloned RIFT working dir, but the clone is upstream main, which does not carry that file yet -- so the build failed with "Could not open requirements file: containers/requirements-container.txt". Stage the host's copy into the image via an Apptainer %files section instead (@@REQFILE@@ -> absolute host path, substituted by build_family.sh), and install from /opt/requirements-container.txt. The build no longer depends on the cloned branch shipping the file. Comments/README updated to match (the top-level rift_container.def keeps its equivalent inline list for the default build). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…er builds Unprivileged apptainer builds on shared clusters (e.g. CIT) fall back to the proot engine, whose mksquashfs step is blocked by seccomp: proot error: ptrace(TRACEME): Operation not permitted mksquashfs command failed build_family.sh now exports PROOT_NO_SECCOMP=1 (overridable, harmless when proot is unused) -- the documented workaround. README gains a build-troubleshooting note covering this plus the better --fakeroot / userns alternatives and APPTAINER_TMPDIR for space. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mksquashfs) PROOT_NO_SECCOMP=1 silenced the seccomp message but proot still cannot exec mksquashfs on locked-down clusters (CIT), so SIF creation kept failing. The actual fixes avoid the proot engine: - build_family.sh now accepts --fakeroot (pass-through to apptainer build; the IGWN/CIT-recommended path, needs subuid/subgid + userns), --sandbox (build a directory, skipping mksquashfs entirely), and passes any other --flag through. Empty-array expansion made safe for RHEL7-era bash under set -u. - README + docs troubleshooting rewritten to lead with --fakeroot / --sandbox / build-elsewhere; PROOT_NO_SECCOMP demoted to best-effort. Validated on cardassia: --fakeroot produces a .sif and --sandbox a directory with a tiny test image; flag/command construction verified via a stub apptainer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…equirement A single SINGULARITY_BASE_EXE_DIR is applied to every image the selection expression can pick (ILE/CIP locate the executable as SINGULARITY_BASE_EXE_DIR + <exe name>, no per-image override). So all images in a manifest must install RIFT executables at the same in-container path / share a common layout -- don't mix apples and oranges. Documented in the prototype manifest, the generated-manifest stub, the README schema, and the docs page (also flags SINGULARITY_BASE_EXE_DIR_HYPERPIPE). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.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
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.