Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

t.rast.aggregate: Adjustments to temporally weighted aggregation #1799

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
75 changes: 66 additions & 9 deletions python/grass/temporal/aggregation.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ def aggregate_by_topology(
nprocs=1,
spatial=None,
dbif=None,
weighting=False,
overwrite=False,
file_limit=1000,
):
Expand Down Expand Up @@ -258,7 +259,7 @@ def aggregate_by_topology(

msgr = get_tgis_message_interface()

dbif, connection_state_changed = init_dbif(dbif)
dbif, connected = init_dbif(dbif)
wenzeslaus marked this conversation as resolved.
Show resolved Hide resolved

topo_builder = SpatioTemporalTopologyBuilder()
topo_builder.build(mapsA=granularity_list, mapsB=map_list, spatial=spatial)
Expand Down Expand Up @@ -316,6 +317,47 @@ def aggregate_by_topology(
if "overlapped" in topo_list and granule.overlapped:
for map_layer in granule.overlapped:
aggregation_list.append(map_layer.get_name())
if "related" in topo_list:
aggregation_weights = []
set_list = set()
try:
for map_layer in granule.overlaps:
set_list.add(map_layer)
except:
pass
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be similar to the code above, but it is written in a completely different style. Any reason not to follow the above style? Additionally, the try-except is unclear as it is written now since it is a bare except. See Flake8: ./temporal/aggregation.py:326:13: E722 do not use bare 'except' (also in CI).

try:
for map_layer in granule.overlapped:
set_list.add(map_layer)
except:
pass
try:
for map_layer in granule.contains:
set_list.add(map_layer)
except:
pass
try:
for map_layer in granule.equal:
set_list.add(map_layer)
except:
pass
try:
for map_layer in granule.during:
set_list.add(map_layer)
except:
pass
if set_list:
for map_layer in set_list:
aggregation_list.append(map_layer.get_name())
t_granule_contained = map_layer.get_absolute_time()
t_granule = granule.get_absolute_time()
if None in t_granule_contained or None in t_granule:
aggregation_weights.append(0)
else:
overlap_abs = min(t_granule[1], t_granule_contained[1]) - max(
t_granule[0], t_granule_contained[0]
)
Copy link
Member

Choose a reason for hiding this comment

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

(In-code) comments for these lines would be appreciated. Easier to review, easier to maintain when the intention is documented.

overlap_rel = overlap_abs / (t_granule[1] - t_granule[0])
aggregation_weights.append(overlap_rel)

if aggregation_list:
msgr.verbose(
Expand Down Expand Up @@ -360,11 +402,18 @@ def aggregate_by_topology(
# Create the r.series input file
filename = gscript.tempfile(True)
file = open(filename, "w")
for name in aggregation_list:
string = "%s\n" % (name)
file.write(string)
file.close()

if weighting:
for i, name in enumerate(aggregation_list):
string = name
weight = aggregation_weights[i]
file.write("%s|%f\n" % (string, weight))
file.close()
Copy link
Member

Choose a reason for hiding this comment

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

The file.close() call is duplicated in the branches and does not need to be. Since you are rewriting most of the file code here, do you want to just use with statement avoiding the need for explicit close() call?

else:
for name in aggregation_list:
string = "%s\n" % (name)
file.write(string)
file.close()
# Perform aggregation
mod = copy.deepcopy(r_series)
mod(file=filename, output=output_name)
if len(aggregation_list) > int(file_limit):
Expand All @@ -380,13 +429,21 @@ def aggregate_by_topology(
mod(flags="z")
process_queue.put(mod)
else:
mod = copy.deepcopy(g_copy)
mod(raster=[aggregation_list[0], output_name])
if weighting:
mod = copy.deepcopy(r_series)
mod(
input=aggregation_list[0],
output=output_name,
weights=aggregation_weights[0],
)
else:
mod = copy.deepcopy(g_copy)
mod(raster=[aggregation_list[0], output_name])
process_queue.put(mod)

process_queue.wait()

if connection_state_changed:
if connected:
dbif.close()

msgr.percent(1, 1, 1)
Expand Down
15 changes: 12 additions & 3 deletions temporal/t.rast.aggregate/t.rast.aggregate.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#!/usr/bin/env python3

############################################################################
neteler marked this conversation as resolved.
Show resolved Hide resolved
#
# MODULE: t.rast.aggregate
Expand All @@ -20,7 +19,7 @@
#
#############################################################################

# %module
#%module
neteler marked this conversation as resolved.
Show resolved Hide resolved
neteler marked this conversation as resolved.
Show resolved Hide resolved
# % description: Aggregates temporally the maps of a space time raster dataset by a user defined granularity.
# % keyword: temporal
# % keyword: aggregation
Expand Down Expand Up @@ -99,7 +98,7 @@
# %end

# %option G_OPT_T_SAMPLE
# % options: equal,overlaps,overlapped,starts,started,finishes,finished,during,contains
# % options: equal,overlaps,overlapped,starts,started,finishes,finished,during,contains,related
# % answer: contains
# %end

Expand All @@ -111,6 +110,11 @@
# % description: Register Null maps
# %end

# %flag
# % key: w
# % description: Aggregation weighted by temporal overlap between input rasters and rasters of defined granularity
# %end

import grass.script as gcore


Expand All @@ -128,13 +132,17 @@ def main():
gran = options["granularity"]
base = options["basename"]
register_null = flags["n"]
weighting = flags["w"]
method = options["method"]
sampling = options["sampling"]
offset = options["offset"]
nprocs = options["nprocs"]
file_limit = options["file_limit"]
time_suffix = options["suffix"]

if weighting and not sampling == "related":
gcore.fatal(_("Weighting only works with method 'related'"))
wenzeslaus marked this conversation as resolved.
Show resolved Hide resolved

topo_list = sampling.split(",")

tgis.init()
Expand Down Expand Up @@ -203,6 +211,7 @@ def main():
method=method,
nprocs=nprocs,
spatial=None,
weighting=weighting,
overwrite=gcore.overwrite(),
file_limit=file_limit,
)
Expand Down