Fix ground truthing CRS regression and extract nps_active_space/setup - #102
Merged
Merged
Conversation
Use microphone.crs from get_deployment() when launching the ground-truthing app instead of passing the coords_to_utm() tuple. Co-authored-by: Cursor <cursoragent@cursor.com>
Move DEM clipping, GridFloat output, site directory creation, and setup_site orchestration into a library package. Thin project_setup.py to argparse and config only. Deduplicate site directory and .sit writing in ActiveSpaceGenerator. Add dem_elevation_units config and setup integration tests. Co-authored-by: Cursor <cursoragent@cursor.com>
…tion. Write and read .sit easting/northing in the NMSIM project UTM zone (western study-area edge). Split site helpers into site_writer and site_decoder; fail clearly on legacy mic-local zone files with a project_setup rerun command. Add migrate_project_sites.py for in-place .sit rewrites and cross-zone regression tests. Co-authored-by: Cursor <cursoragent@cursor.com>
elliott-ruebush
force-pushed
the
fix/ground-truthing-crs-tuple-regression
branch
from
July 22, 2026 00:34
fcac5f2 to
ad82b48
Compare
elliott-ruebush
commented
Jul 22, 2026
No active callers use the PR #75 create_NMSIM_* names; project_setup and tests already import the canonical symbols. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep __init__ as a thin entry point for setup_site and NMSIM_DST_CRS; import site helpers from submodules. Use read_sit_file() instead of re-parsing .sit lines in helpers. Co-authored-by: Cursor <cursoragent@cursor.com>
dbetchkal
approved these changes
Jul 24, 2026
dbetchkal
left a comment
Owner
There was a problem hiding this comment.
Excellent work! Despite some nuance, this is clear to read and makes geographic sense.
| nvspl=nvspl, | ||
| mic=microphone, | ||
| crs=coords_to_utm(microphone.lat, microphone.lon), | ||
| crs=microphone.crs, |
|
|
||
| with rasterio.open(output_tif) as ds: | ||
| values = ds.read(1, masked=True) | ||
| assert float(values.mean()) == pytest.approx(expected_mean, abs=1.0) |
Owner
There was a problem hiding this comment.
Nice! pytest.approx seems very useful
| assert float(hdr_lines["cellsize"]) == pytest.approx(dst_transform.a) | ||
|
|
||
| flt = np.fromfile(output_base.with_suffix(".flt"), dtype=np.float32).reshape(2, 2) | ||
| assert flt[0, 0] == pytest.approx(100.0) |
Owner
There was a problem hiding this comment.
As I read this, line 211 initializes a 2x2 matrix and here we simply test each cell to see if the correct values were written to the .flt, right?
elliott-ruebush
added a commit
that referenced
this pull request
Jul 28, 2026
Keep pyproject README structure and document dem_elevation_units in template.config comments after merging setup changes from #102. Co-authored-by: Cursor <cursoragent@cursor.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.
Summary
#75regression inrun_ground_truthing.py: usemicrophone.crsfromget_deployment()instead of thecoords_to_utm()tupleproject_setup.pyintonps_active_space/setup/(site_writer.py,site_decoder.py,elevation.py,workflow.py); CLI is argparse + config only.sitmicrophone coords to NMSIM project UTM zone (western study-area edge); fixes cross-zone deployments (e.g. DENABULL).sitfiles:get_deployment()raises with a copy-pasteproject_setupcommand;migrate_project_sites.pyrewrites on-disk.sitfiles without full re-setupActiveSpaceGeneratorsite dir /.sitwriting via shared setup helpersContext
PR #75 changed
coords_to_utmto returntuple[str, int]. This PR fixes the missed ground-truthing caller and refactors project setup. PR #84 will need the newcoords_to_utmsignature.Test plan
pytest tests/setup/(16 passed)pytest(96 passed).sit/study areaproject_setup.pyagainst a configured environmentmigrate_project_sites.py --dry-runon legacy.sitif availableExample Creation of Bull River site folder
Migration script dry run on current `T:\ResMgmt\Sound\NMSim_Partial_Extract\NMSim\01 SITES` contents
Follow-ups (future PR?)
Already deduped here:
create_site_dir,write_site_file.Remaining
ActiveSpaceGeneratoroverlap:setup.create_elevation_tif(rasterio, feet→meters, int16) vs_mask_dem_file(GDAL warp, no unit conversion)setup.write_gridfloatvs_create_dem_flt.sit:_create_site_filestill writes mic-local UTM coords; onlyproject_setup/setup_siteuses project zone todayOther: NLCD landcover setup, flexible study-area ingestion.
Question:
Is it intended that the active space generator
_mask_dem_filefunction does not convert feet-> meters? maybe I'm missing something here