Draft
Conversation
src.sample() gives an array per point, even for a single band, so we still need s[0] to extract the scalar value
Contributor
Author
|
I've addressed all suggestions. The main changes are:
|
Typer only supports auto-generated --long-form options OR explicit custom names, not both. Dropping short flags (-c, -v, -t, -i, -l, -o, -d) lets Typer derive intuitive --option-names from parameter names without cluttering the code with redundant explicit strings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously each worker process created its own tqdm bar inside compute_spatial_adjustment_at_points(), causing rapid flickering between competing progress displays. Now progress is controlled by the caller via an optional progress_bar parameter. Sequential path: one smooth bar covering geology + terrain. Parallel path: splits into many small chunks instead of n_proc large chunks, with a single parent bar updating as each chunk completes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… grid updates Add Geology/Terrain prefix to all progress bars so users understand why stages run twice. Use many small chunks for grid spatial adjustment so the progress bar updates smoothly. Suppress progress bar for single-chunk bbox search in favor of a simple status message. Split points sequential path into separate geology and terrain progress bars. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
Getting Vs30 values at Cesar's list of locations has proven to be a very useful stress test. Actually using the CLI tool revealed that the CLI flags were confusing and progress bars unhelpful, so I've improved those now. Unfortunately, (or perhaps actually fortunately), I also learned that when doing lists of points (rather than a grid), the coastal distance adjustment was not being properly applied, so that will probably take a few days to fix. I'll mark this PR as a draft until it's fixed. |
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.
This PR is a major refactor of the Vs30 package to improve readability, modularity, and testability. The mathematical expressions are equivalent to the original version, but use clearer variable names and simplified forms. Regression tests confirm that the output from the refactored codebase is consistent with the original version.
Design changes
Centralized configuration
config.yamlfile, rather than being hardcoded across various source files as in the original codebase.Modular CLI
vs30) exposes the full pipeline as well as individual stages (update-categorical-vs30-models,make-initial-vs30-raster,spatial-fit, etc.), making it possible to run or re-run specific steps independently.compute-at-locationscommand computes Vs30 at specific latitude/longitude points without generating full raster grids, which is efficient for querying a small number of sites.Input Vs30 data
"original"combined three hardcoded sources with dataset-specific filtering and downsampling;"cpt"loaded CPT data), with different processing and Bayesian update paths for each mode.Vs30 map generation
max_pointsparameter caps the number of nearest observations used for the per-pixel MVN update, limiting the cost of the matrix inversion at each pixel.Multiprocessing
compute-at-locationssupport multiprocessing. The spatial adjustment work is divided into chunks of affected pixels distributed across worker processes.threadpoolctl, rather than requiring environment variables to be set before import.Test suite