Skip to content

r.in.pdal: Preserve computational region resolution with -d flag#6923

Open
saurabh12nxf wants to merge 8 commits intoOSGeo:mainfrom
saurabh12nxf:fix-r-in-pdal-base-raster-resolution
Open

r.in.pdal: Preserve computational region resolution with -d flag#6923
saurabh12nxf wants to merge 8 commits intoOSGeo:mainfrom
saurabh12nxf:fix-r-in-pdal-base-raster-resolution

Conversation

@saurabh12nxf
Copy link
Contributor

Description

Fixes #6740

The -d flag in r.in.pdal was incorrectly changing the output raster resolution to match the base raster resolution, instead of only using the base raster's resolution for reading values.

Changes

  • Removed lines 665-670 in raster/r.in.pdal/main.cpp that incorrectly modified the output region resolution
  • The -d flag now correctly only affects the input window for reading base raster values, matching the behavior of r.in.lidar

Expected Behavior

When using -d flag with a 30m computational region and a 1m base raster:

  • Before: Output raster at 1m resolution (incorrect)
  • After: Output raster at 30m resolution (correct)

The base raster is still read at its native 1m resolution for Z value adjustments, but the statistical aggregation and output raster creation happens at the 30m computational region resolution.

Testing

Tested with:

  • Computational region: 30m
  • Base raster: 1m DEM
  • Method: max
  • zrange: 0-75m

Result: Output raster correctly created at 30m resolution with Z values adjusted using 1m base raster.

@github-actions github-actions bot added raster Related to raster data processing C++ Related code is in C++ module labels Jan 19, 2026
@saurabh12nxf saurabh12nxf changed the title Fix r.in.pdal: Preserve computational region resolution with -d flag r.in.pdal: Preserve computational region resolution with -d flag Jan 19, 2026
@marisn
Copy link
Contributor

marisn commented Jan 20, 2026

Please add a test.

@github-actions github-actions bot added Python Related code is in Python tests Related to Test Suite labels Jan 21, 2026
@saurabh12nxf
Copy link
Contributor Author

@marisn Added comprehensive test in test_r_in_pdal_base_raster_resolution.py.

The test verifies that the -d flag preserves computational region resolution
while reading base raster at its native resolution.

Test scenario: 6m computational region + 2m base raster → output is 6m (not 2m).
This test would fail before the fix and passes after.

@saurabh12nxf
Copy link
Contributor Author

@marisn I see some CI failures but I'm having trouble identifying if they're related to my test. Could you help me understand what's failing? The test I added is straightforward and validates the resolution preservation.

@echoix
Copy link
Member

echoix commented Jan 21, 2026

@marisn I see some CI failures but I'm having trouble identifying if they're related to my test. Could you help me understand what's failing? The test I added is straightforward and validates the resolution preservation.

Yes they seem related. At least for macOS, a failure in the r.in.pdal in a test related to computational region

@saurabh12nxf
Copy link
Contributor Author

Hi @marisn & @echoix The test is now passing! The only remaining CI failures are:

  1. i.smap test - Unrelated to my changes (pre-existing flaky test with value mismatch)
  2. Build & Test Result - Summary check that fails because of i.smap

My test (test_r_in_pdal_base_raster_resolution) is passing on all platforms. The fix is working correctly!

please once verify it .

@echoix
Copy link
Member

echoix commented Jan 22, 2026

Launched a rerun, i.smap is a flaky test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++ Related code is in C++ module Python Related code is in Python raster Related to raster data processing tests Related to Test Suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Change in function between r.in.lidar and r.in.pdal for use base raster resolution

3 participants