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
192 changes: 101 additions & 91 deletions temporal/t.rast.aggregate/t.rast.aggregate.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#!/usr/bin/env python3

# -*- coding: utf-8 -*-
neteler marked this conversation as resolved.
Show resolved Hide resolved
neteler marked this conversation as resolved.
Show resolved Hide resolved
neteler marked this conversation as resolved.
Show resolved Hide resolved
############################################################################
neteler marked this conversation as resolved.
Show resolved Hide resolved
#
# MODULE: t.rast.aggregate
Expand All @@ -20,96 +20,101 @@
#
#############################################################################

# %module
# % description: Aggregates temporally the maps of a space time raster dataset by a user defined granularity.
# % keyword: temporal
# % keyword: aggregation
# % keyword: raster
# % keyword: time
# %end

# %option G_OPT_STRDS_INPUT
# %end

# %option G_OPT_STRDS_OUTPUT
# %end

# %option
# % key: basename
# % type: string
# % label: Basename of the new generated output maps
# % description: Either a numerical suffix or the start time (s-flag) separated by an underscore will be attached to create a unique identifier
# % required: yes
# % multiple: no
# % gisprompt:
# %end

# %option
# % key: suffix
# % type: string
# % description: Suffix to add at basename: set 'gran' for granularity, 'time' for the full time format, 'num' for numerical suffix with a specific number of digits (default %05)
# % answer: gran
# % required: no
# % multiple: no
# %end

# %option
# % key: granularity
# % type: string
# % description: Aggregation granularity, format absolute time "x years, x months, x weeks, x days, x hours, x minutes, x seconds" or an integer value for relative time
# % required: yes
# % multiple: no
# %end

# %option
# % key: method
# % type: string
# % description: Aggregate operation to be performed on the raster maps
# % required: yes
# % multiple: no
# % options: average,count,median,mode,minimum,min_raster,maximum,max_raster,stddev,range,sum,variance,diversity,slope,offset,detcoeff,quart1,quart3,perc90,quantile,skewness,kurtosis
# % answer: average
# %end

# %option
# % key: offset
# % type: integer
# % description: Offset that is used to create the output map ids, output map id is generated as: basename_ (count + offset)
# % required: no
# % multiple: no
# % answer: 0
# %end

# %option
# % key: nprocs
# % type: integer
# % description: Number of r.series processes to run in parallel
# % required: no
# % multiple: no
# % answer: 1
# %end

# %option
# % key: file_limit
# % type: integer
# % description: The maximum number of open files allowed for each r.series process
# % required: no
# % multiple: no
# % answer: 1000
# %end

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

# %option G_OPT_T_WHERE
# %end

# %flag
# % key: n
# % description: Register Null maps
# %end
#%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
#% keyword: raster
#% keyword: time
#%end

#%option G_OPT_STRDS_INPUT
#%end

#%option G_OPT_STRDS_OUTPUT
#%end

#%option
#% key: basename
#% type: string
#% label: Basename of the new generated output maps
#% description: Either a numerical suffix or the start time (s-flag) separated by an underscore will be attached to create a unique identifier
#% required: yes
#% multiple: no
#% gisprompt:
#%end

#%option
#% key: suffix
#% type: string
#% description: Suffix to add at basename: set 'gran' for granularity, 'time' for the full time format, 'num' for numerical suffix with a specific number of digits (default %05)
#% answer: gran
#% required: no
#% multiple: no
#%end

#%option
#% key: granularity
#% type: string
#% description: Aggregation granularity, format absolute time "x years, x months, x weeks, x days, x hours, x minutes, x seconds" or an integer value for relative time
#% required: yes
#% multiple: no
#%end

#%option
#% key: method
#% type: string
#% description: Aggregate operation to be performed on the raster maps
#% required: yes
#% multiple: no
#% options: average,count,median,mode,minimum,min_raster,maximum,max_raster,stddev,range,sum,variance,diversity,slope,offset,detcoeff,quart1,quart3,perc90,quantile,skewness,kurtosis
#% answer: average
#%end

#%option
#% key: offset
#% type: integer
#% description: Offset that is used to create the output map ids, output map id is generated as: basename_ (count + offset)
#% required: no
#% multiple: no
#% answer: 0
#%end

#%option
#% key: nprocs
#% type: integer
#% description: Number of r.series processes to run in parallel
#% required: no
#% multiple: no
#% answer: 1
#%end

#%option
#% key: file_limit
#% type: integer
#% description: The maximum number of open files allowed for each r.series process
#% required: no
#% multiple: no
#% answer: 1000
#%end

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

#%option G_OPT_T_WHERE
#%end

#%flag
#% key: n
#% description: Register Null maps
#%end

#%flag
#% key: w
#% description: Aggregation weighted by temporal overlap between input rasters and rasters of defined granularity
#%end
neteler marked this conversation as resolved.
Show resolved Hide resolved
neteler marked this conversation as resolved.
Show resolved Hide resolved
neteler marked this conversation as resolved.
Show resolved Hide resolved

import grass.script as gcore

Expand All @@ -128,13 +133,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 +212,7 @@ def main():
method=method,
nprocs=nprocs,
spatial=None,
weighting=weighting,
overwrite=gcore.overwrite(),
file_limit=file_limit,
)
Expand Down