Skip to content

Sanitize Copernicus DEM samples, fix tile overlap logic, preserve 2-node highways, and allow env example files#6

Open
tronpis wants to merge 2 commits into
codex/fix-.gitignore-entries-and-llbbox-validationfrom
codex/update-comment-for-image-data-handling
Open

Sanitize Copernicus DEM samples, fix tile overlap logic, preserve 2-node highways, and allow env example files#6
tronpis wants to merge 2 commits into
codex/fix-.gitignore-entries-and-llbbox-validationfrom
codex/update-comment-for-image-data-handling

Conversation

@tronpis
Copy link
Copy Markdown
Owner

@tronpis tronpis commented May 11, 2026

Motivation

  • Ensure elevation values read from Copernicus GeoTIFFs are sane and treat common nodata/sentinel values as NaN to avoid corrupting downstream grids.
  • Correct tile selection so 1° tiles are chosen using proper inclusive/exclusive integer-edge semantics and clamp to global bounds.
  • Preserve clipped road geometries that legitimately have only two nodes while still filtering out degenerate non-road geometries.
  • Allow committing example environment files by excluding !.env.example and !.env.*.example from .gitignore.

Description

  • Added normalize_elevation_sample to filter out non-finite and out-of-range elevation samples and replaced raw conversions with calls to this function when decoding TIFF samples.
  • Introduced overlapping_tile_range and overlapping_tiles to compute overlapping 1°×1° tile coordinates using proper floor/ceil logic and clamping to [-90,89]/[-180,179] limits, and replaced the previous naive loops with overlapping_tiles.
  • Added MIN_VALID_ELEVATION_M and MAX_VALID_ELEVATION_M sanity bounds and updated TIFF decoding paths to handle both F32 and F64 decoding results while normalizing values.
  • Small logging/string formatting tweaks around HTTP errors and tile name formatting.
  • Implemented min_clipped_node_count to return 2 for ways tagged as highways and 3 otherwise, and used it to decide whether a clipped ProcessedWay should be kept.
  • Updated .gitignore to allow committing env example files via !.env.example and !.env.*.example entries.
  • Added unit tests for normalize_elevation_sample, overlapping_tiles, and min_clipped_node_count and kept existing provider and parser tests.

Testing

  • Ran cargo test which executed provider and parser unit tests including the newly added tests and they all passed.
  • New tests include test_normalize_elevation_sample_keeps_valid_values, test_normalize_elevation_sample_filters_invalid_values, test_overlapping_tiles_excludes_integer_max_edge, test_overlapping_tiles_clamps_global_edges, and test_min_clipped_node_count_keeps_two_node_roads, and they succeeded.

Codex Task

@qodo-code-review
Copy link
Copy Markdown

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

Copy link
Copy Markdown

@amazon-q-developer amazon-q-developer Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All changes in this PR are well-implemented and thoroughly tested. The elevation sanitization logic correctly handles nodata values, the tile overlap algorithm properly clamps to global bounds, and the minimum node count logic appropriately preserves 2-node highways while filtering degenerate geometries. The comprehensive unit tests validate all the critical functionality.


You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the .gitignore to include environment example files and enhances the Copernicus DEM provider by implementing elevation normalization and a more precise tile overlap calculation. Additionally, it adjusts Overture data processing to allow two-node road segments while maintaining a three-node minimum for other features. Review feedback suggests optimizing the tile range logic by using inclusive ranges to improve efficiency and handle edge cases where the bounding box is outside valid bounds.

Comment thread src/elevation/providers/copernicus.rs Outdated
Comment on lines +37 to +44
fn overlapping_tiles(bbox: &LLBBox) -> Vec<(i32, i32)> {
let lats = overlapping_tile_range(bbox.min().lat(), bbox.max().lat(), -90, 89);
let lngs = overlapping_tile_range(bbox.min().lng(), bbox.max().lng(), -180, 179);

lats.into_iter()
.flat_map(|lat| lngs.iter().copied().map(move |lng| (lat, lng)))
.collect()
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since overlapping_tile_range now returns a range, overlapping_tiles can be simplified to avoid intermediate Vec allocations for latitudes and longitudes. Note that we still collect() the final result because the caller in fetch_raw (line 104) requires the length of the tile list for progress reporting.

fn overlapping_tiles(bbox: &LLBBox) -> Vec<(i32, i32)> {
    let lats = overlapping_tile_range(bbox.min().lat(), bbox.max().lat(), -90, 89);
    let lngs = overlapping_tile_range(bbox.min().lng(), bbox.max().lng(), -180, 179);

    lats.flat_map(|lat| lngs.clone().map(move |lng| (lat, lng)))
        .collect()
}

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant