Skip to content

Commit

Permalink
Merge pull request #807 from kordejong/gh800
Browse files Browse the repository at this point in the history 
Prevent to_gdal to write outside of the raster's shape
  • Loading branch information
@kordejong
kordejong authored Feb 6, 2025
2 parents 78b92b2 + 0f8cac8 commit 31e0342
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 34 deletions.
62 changes: 38 additions & 24 deletions source/framework/io/source/gdal.cpp.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,14 +202,33 @@ namespace lue {
auto instantiate(
InputNoDataPolicy const indp,
OutputNoDataPolicy const ondp,
Offset const& offset,
Offset const& hyperslab_offset,
Offset const& partition_offset,
Shape const& shape) -> Partition
{
AnnotateFunction const annotate{"read_partition"};

// - hyperslab_offset contains the number of cells to offset the partitions to read by
// - partition_offsets contains the offset per partition to read, relative to the
// hyperslab_offset
//
// → When reading a partition, both offsets must be summed
// → When creating a partition, only partition_offset must be used

Data data{shape};

_band_ptr->read_partition(offset, data);
{
// Offset within the raster to read the partition from
Offset offset{hyperslab_offset};

std::transform(offset.begin(), offset.end(), partition_offset.begin(), offset.begin(),
[](auto offset1, auto const offset2)
{
return offset1 + offset2;
});

_band_ptr->read_partition(offset, data);
}

std::transform(
data.begin(),
Expand All @@ -225,7 +244,7 @@ namespace lue {
return value;
});

return Partition{hpx::find_here(), offset, std::move(data)};
return Partition{hpx::find_here(), partition_offset, std::move(data)};
}

private:
Expand Down Expand Up @@ -254,15 +273,16 @@ namespace lue {
auto read_partition_per_locality(
Policies const& policies,
std::string const& name,
std::vector<lue::OffsetT<Partition>> const& offsets,
lue::OffsetT<Partition> const& hyperslab_offset,
std::vector<lue::OffsetT<Partition>> const& partition_offsets,
std::vector<lue::ShapeT<Partition>> const& partition_shapes) -> std::vector<Partition>
{
gdal::DatasetPtr dataset_ptr{gdal::open_dataset(name, ::GA_ReadOnly)};
gdal::RasterBandPtr band_ptr{gdal::raster_band(*dataset_ptr)};

using Element = lue::ElementT<Partition>;

std::size_t const nr_partitions{std::size(offsets)};
std::size_t const nr_partitions{std::size(partition_offsets)};
std::vector<Partition> partitions(nr_partitions);

auto indp{std::get<0>(policies.inputs_policies()).input_no_data_policy()};
Expand All @@ -279,9 +299,10 @@ namespace lue {
[indp,
ondp,
partition_reader,
offset = offsets[0],
hyperslab_offset = hyperslab_offset,
partition_offset = partition_offsets[0],
partition_shape = partition_shapes[0]]() mutable
{ return partition_reader.instantiate(indp, ondp, offset, partition_shape); });
{ return partition_reader.instantiate(indp, ondp, hyperslab_offset, partition_offset, partition_shape); });

for (std::size_t idx = 1; idx < nr_partitions; ++idx)
{
Expand All @@ -290,9 +311,10 @@ namespace lue {
[indp,
ondp,
partition_reader,
offset = offsets[idx],
hyperslab_offset = hyperslab_offset,
partition_offset = partition_offsets[idx],
partition_shape = partition_shapes[idx]](auto const& /* previous_partition */) mutable
{ return partition_reader.instantiate(indp, ondp, offset, partition_shape); }
{ return partition_reader.instantiate(indp, ondp, hyperslab_offset, partition_offset, partition_shape); }

);
}
Expand Down Expand Up @@ -327,10 +349,10 @@ namespace lue {
static constexpr bool instantiate_per_locality{true};


ReadPartitionsPerLocality(std::string name, Hyperslab<rank>::Offsets const& offsets={0, 0}):
ReadPartitionsPerLocality(std::string name, Hyperslab<rank>::Offsets const& hyperslab_offset={0, 0}):

_name{std::move(name)},
_offsets{offsets}
_hyperslab_offset{hyperslab_offset}

{
}
Expand All @@ -341,30 +363,22 @@ namespace lue {
hpx::id_type const locality_id,
[[maybe_unused]] Policies const& policies,
[[maybe_unused]] Shape const& array_shape,
std::vector<Offset> offsets,
std::vector<Offset> partition_offsets,
std::vector<Shape> partition_shapes) -> hpx::future<std::vector<Partition>>
{
using Action = ReadPartitionsPerLocalityAction<Policies, Partition>;
Action action{};

for(auto& offset: offsets)
{
std::transform(_offsets.begin(), _offsets.end(), offset.begin(), offset.begin(),
[](auto offset1, auto const offset2)
{
return offset1 + offset2;
});
}

return hpx::async(

[locality_id,
action = std::move(action),
policies,
name = _name,
offsets = std::move(offsets),
hyperslab_offset = _hyperslab_offset,
partition_offsets = std::move(partition_offsets),
partition_shapes = std::move(partition_shapes)]()
{ return action(locality_id, policies, name, offsets, partition_shapes); }
{ return action(locality_id, policies, name, hyperslab_offset, partition_offsets, partition_shapes); }

);
}
Expand All @@ -374,7 +388,7 @@ namespace lue {
std::string _name;

// Offset of all partitions within the raster
Hyperslab<rank>::Offsets _offsets;
Hyperslab<rank>::Offsets _hyperslab_offset;
};


Expand Down
78 changes: 70 additions & 8 deletions source/framework/io/test/gdal_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "lue/framework.hpp"
#include "lue/gdal.hpp"
#include <hpx/config.hpp>
#include <random>


BOOST_AUTO_TEST_CASE(array_all_valid)
Expand All @@ -22,7 +23,7 @@ BOOST_AUTO_TEST_CASE(array_all_valid)
Array array_written{lue::create_partitioned_array<Element>(array_shape, partition_shape, fill_value)};
std::string const name{"lue_framework_io_gdal_array_int_all_valid.tif"};

lue::to_gdal<Element>(array_written, name).wait();
lue::to_gdal<Element>(array_written, name).get();

BOOST_CHECK(lue::gdal::try_open_dataset(name, GDALAccess::GA_ReadOnly));

Expand All @@ -44,7 +45,7 @@ BOOST_AUTO_TEST_CASE(array_none_valid)
Array array_written{lue::create_partitioned_array<Element>(array_shape, partition_shape, fill_value)};
std::string const name{"lue_framework_io_gdal_array_int_none_valid.tif"};

lue::to_gdal<Element>(array_written, name).wait();
lue::to_gdal<Element>(array_written, name).get();

BOOST_CHECK(lue::gdal::try_open_dataset(name, GDALAccess::GA_ReadOnly));

Expand All @@ -66,7 +67,7 @@ BOOST_AUTO_TEST_CASE(array_float_all_valid)
Array array_written{lue::create_partitioned_array<Element>(array_shape, partition_shape, fill_value)};
std::string const name{"lue_framework_io_gdal_array_float_all_valid.tif"};

lue::to_gdal<Element>(array_written, name).wait();
lue::to_gdal<Element>(array_written, name).get();

BOOST_CHECK(lue::gdal::try_open_dataset(name, GDALAccess::GA_ReadOnly));

Expand All @@ -88,7 +89,7 @@ BOOST_AUTO_TEST_CASE(array_float_none_valid)
Array array_written{lue::create_partitioned_array<Element>(array_shape, partition_shape, fill_value)};
std::string const name{"lue_framework_io_gdal_array_float_none_valid.tif"};

lue::to_gdal<Element>(array_written, name).wait();
lue::to_gdal<Element>(array_written, name).get();

BOOST_CHECK(lue::gdal::try_open_dataset(name, GDALAccess::GA_ReadOnly));

Expand Down Expand Up @@ -122,8 +123,8 @@ BOOST_AUTO_TEST_CASE(hyperslab)
std::string const row_idxs_name{"lue_framework_io_gdal_hyperslab_row_idxs.tif"};
std::string const col_idxs_name{"lue_framework_io_gdal_hyperslab_col_idxs.tif"};

lue::to_gdal(row_idxs_written, row_idxs_name).wait();
lue::to_gdal(col_idxs_written, col_idxs_name).wait();
lue::to_gdal(row_idxs_written, row_idxs_name).get();
lue::to_gdal(col_idxs_written, col_idxs_name).get();

Hyperslab const hyperslab{{30, 20}, {20, 10}};

Expand Down Expand Up @@ -162,7 +163,7 @@ BOOST_AUTO_TEST_CASE(valid_hyperslab)

std::string const idxs_name{"lue_framework_io_gdal_wrong_hyperslab_idxs.tif"};

lue::to_gdal(idxs_written, idxs_name).wait();
lue::to_gdal(idxs_written, idxs_name).get();

{
// Hyperslab corresponds with whole array
Expand Down Expand Up @@ -190,7 +191,7 @@ BOOST_AUTO_TEST_CASE(incorrect_hyperslab)

std::string const idxs_name{"lue_framework_io_gdal_wrong_hyperslab_idxs.tif"};

lue::to_gdal(idxs_written, idxs_name).wait();
lue::to_gdal(idxs_written, idxs_name).get();

{
// Empty hyperslab
Expand Down Expand Up @@ -233,3 +234,64 @@ BOOST_AUTO_TEST_CASE(incorrect_hyperslab)
{ return std::string(exception.what()).find("extents beyond array") != std::string::npos; });
}
}


BOOST_AUTO_TEST_CASE(round_trip)
{
// Random array shapes, random partition shapeѕ, all valid. Verify that no exception is thrown.
using Element = lue::LargestIntegralElement;
lue::Rank const rank{2};
using Shape = lue::Shape<lue::Count, rank>;
using Array = lue::PartitionedArray<Element, rank>;
using Hyperslab = lue::Hyperslab<2>;

std::random_device random_device{};
std::default_random_engine random_number_engine(random_device());

auto const random_array_shape = [&]()
{
lue::Count const min{100};
lue::Count const max{500};
std::uniform_int_distribution<lue::Count> distribution(min, max);

return Shape{
distribution(random_number_engine),
distribution(random_number_engine),
};
};

auto const random_partition_shape = [&]()
{
lue::Count const min{40};
lue::Count const max{50};
std::uniform_int_distribution<lue::Count> distribution(min, max);

return Shape{
distribution(random_number_engine),
distribution(random_number_engine),
};
};

auto const random_hyperslab = [&]()
{
lue::Count const min{40};
lue::Count const max{50};
std::uniform_int_distribution<lue::Count> distribution(min, max);

return Hyperslab{{50, 50}, {distribution(random_number_engine), distribution(random_number_engine)}};
};

Element const fill_value{5};
Array array{
lue::create_partitioned_array<Element>(random_array_shape(), random_partition_shape(), fill_value)};

std::string const name_whole{"lue_framework_io_gdal_to_gdal_whole.tif"};
lue::to_gdal<Element>(array, name_whole).get();

auto const hyperslab{random_hyperslab()};
array = lue::from_gdal<Element>(name_whole, hyperslab, lue::default_partition_shape(hyperslab.counts()));

// This used to fail due to a bug in handling hyperslab and partition offsets
std::string const name_subset{"lue_framework_io_gdal_to_gdal_subset.tif"};
lue::to_gdal<Element>(array, name_subset).get();
}
4 changes: 2 additions & 2 deletions source/framework/python/source/gdal/from_gdal.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ namespace lue::framework {
std::optional<pybind11::tuple> const& partition_shape) -> pybind11::object
{
gdal::DatasetPtr dataset{gdal::open_dataset(name, GDALAccess::GA_ReadOnly)};
auto const raster_shape{gdal::shape(*dataset)};
StaticShape<2> static_array_shape{raster_shape[0], raster_shape[1]};
StaticShape<2> static_partition_shape{};

if (partition_shape)
{
auto const raster_shape{gdal::shape(*dataset)};
StaticShape<2> static_array_shape{raster_shape[0], raster_shape[1]};
DynamicShape const dynamic_partition_shape{tuple_to_shape(*partition_shape)};

if (static_array_shape.size() != dynamic_partition_shape.size())
Expand Down

0 comments on commit 31e0342

Please sign in to comment.