Opened 6 years ago

Closed 5 years ago

#860 closed enhancement (fixed)

[raster] Modify ST_MapAlgebra to handle expressions and user-functions

Reported by: dzwarg Owned by: dzwarg
Priority: blocker Milestone: PostGIS 2.0.0
Component: raster Version: trunk
Keywords: history Cc:

Description

  1. Modify ST_MapAlgebra to ST_MapAlgebraExp (with expression) to evaluate cells of a raster with a single expression.
  2. Create ST_MapAlgebraFct (with function) to evaluate cells of a raster with a user-specified plpgsql function.

To support item 2, the rt_pg layer needs a hook from SQL to callback a specified function. I am calling this ST_CallUserFn. This method takes a callback plpgsql function that will be evaluated for every cell in the raster.

A callback plpgsql function currently must be defined in the following manner:

CREATE OR REPLACE FUNCTION MyUserFunctionName(cell FLOAT, VARIADIC args TEXT[])
  RETURNS FLOAT
AS $$
DECLARE
  -- Some custom arguments, always passed in as TEXT type.
  arg1 FLOAT;
  arg2 FLOAT;
BEGIN
  -- To use your custom arguments, cast them to the type you need,
  -- and perform your calculations. Return the new value for the
  -- new raster cell's value.
  RETURN cell;
END;
$$ LANGUAGE plpgsql;

Attachments (4)

callback.patch (3.6 KB) - added by dzwarg 6 years ago.
Added new C function exposed to SQL that will call a cell processing pl/pgsql function. Includes regression tests.
callback_prototype.patch (14.9 KB) - added by dzwarg 6 years ago.
Added prototype SQL for ST_MapAlgebra that accepts user functions.
raster_functions.patch (14.5 KB) - added by dzwarg 6 years ago.
Re-ordered arguments to ST_MapAlgebra, to promote pixeltype before userfunction or expression. Added ST_MapAlgebraFunction.
raster_functions_r6968.patch (89.7 KB) - added by dzwarg 6 years ago.
Updated patch to incorporate all the above changes at r6968 (HEAD at time of writing).

Download all attachments as: .zip

Change History (29)

comment:1 Changed 6 years ago by dzwarg

Status: newassigned

Changed 6 years ago by dzwarg

Attachment: callback.patch added

Added new C function exposed to SQL that will call a cell processing pl/pgsql function. Includes regression tests.

comment:2 Changed 6 years ago by pracine

Priority: mediumblocker

Changed 6 years ago by dzwarg

Attachment: callback_prototype.patch added

Added prototype SQL for ST_MapAlgebra that accepts user functions.

Changed 6 years ago by dzwarg

Attachment: raster_functions.patch added

Re-ordered arguments to ST_MapAlgebra, to promote pixeltype before userfunction or expression. Added ST_MapAlgebraFunction.

Changed 6 years ago by dzwarg

Updated patch to incorporate all the above changes at r6968 (HEAD at time of writing).

comment:3 Changed 6 years ago by pracine

Owner: changed from dzwarg to pracine
Status: assignednew

comment:4 Changed 6 years ago by pracine

Status: newassigned

comment:5 Changed 5 years ago by dzwarg

Owner: changed from pracine to dzwarg
Status: assignednew

comment:6 Changed 5 years ago by dzwarg

Status: newassigned

comment:7 Changed 5 years ago by dzwarg

Changing the sql function to the signature outlined in WKTRaster/SpecificationWorking03 Objective FV.03.1 requires that the pixel type is no longer optional. This would make the rest of the ST_MapAlgebraXXX functions similar when we add user functions and neighborhoods.

comment:8 Changed 5 years ago by pracine

If I remember well we have to move the pixeltype parameter to get the same parameter order of other MapAlgebra? variants allowing to pass extra parameters to the user custom function. Right?

comment:9 Changed 5 years ago by pracine

Regina,

ST_MapAlgebra() is changed to ST_MapAlgebraExpr() to clearly differentiate it from ST_MapAlgebraFct() which will accept a user defined function. Shall we keep a ST_MapAlgebra() wrapper around the simplest expression variant (ST_MapAlgebraExpr())?

The ST_Mapalgebra() series will follow the FV.03 specifications discussed with David in Montreal.

comment:10 Changed 5 years ago by robe

Well one part of me says yes because ST_Mapalgebra is more familiar, but the other part of me says no because I hate aliases. So don't know I guess it's a toss.

comment:11 Changed 5 years ago by robe

I lean more on side of no.

comment:12 in reply to:  8 Changed 5 years ago by dzwarg

Replying to pracine:

If I remember well we have to move the pixeltype parameter to get the same parameter order of other MapAlgebra? variants allowing to pass extra parameters to the user custom function. Right?

Yes, that's correct. The order of the parameter has to change to bring pixeltype in front of the algebra expression, in addition to making pixeltype required. It may still be NULL, but it is no longer an optional parameter. These changes are in r7625, so an update should get you the new versions. I've updated the documentation and all the tests, please let me know if I've missed anything.

comment:13 in reply to:  11 Changed 5 years ago by dzwarg

Replying to robe:

I lean more on side of no.

I've replaced "ST_MapAlgebra" with "ST_MapAlgebraExpr", in the code, debug statements, documentation, and tests.

comment:14 Changed 5 years ago by dzwarg

Keywords: history added

comment:15 Changed 5 years ago by dzwarg

Implemented code in r7980. Onto docs...

comment:16 Changed 5 years ago by dzwarg

Owner: changed from dzwarg to pracine
Status: assignednew

Updated docs in r7984. Reassigning for review before closing.

comment:17 Changed 5 years ago by robe

I think you have a typo in the last example:

shouldn't:

ST_MapAlgebraFct(rast_view,1,NULL,'tan(rast)*rast')

Be:

ST_MapAlgebraFct(rast_view,1,NULL,'rast_plus_tan(float,text[])'::regprocedure')

Also it is unclear from the description what the args argument is for an if all userdefined functions require a second text[]. For example the rast_plus_tan functions takes a text[] as second arg but never uses it.

comment:18 in reply to:  17 Changed 5 years ago by dzwarg

Owner: changed from pracine to dzwarg

Replying to robe:

I think you have a typo in the last example:

shouldn't:

ST_MapAlgebraFct(rast_view,1,NULL,'tan(rast)*rast')

Be:

ST_MapAlgebraFct(rast_view,1,NULL,'rast_plus_tan(float,text[])'::regprocedure')

Also it is unclear from the description what the args argument is for an if all userdefined functions require a second text[]. For example the rast_plus_tan functions takes a text[] as second arg but never uses it.

Yup! That's a typo.

I will add a paragraph describing what a userfunction should look like, what the arguments are, and things of that nature.

comment:19 Changed 5 years ago by dzwarg

Owner: changed from dzwarg to pracine

Fixed typo. Added more documentation for the requirements of user functions and variadic text args to ST_MapAlgebraFct. Reassigning for review.

comment:20 Changed 5 years ago by robe

I made some more corrections at r7988. There is still something wrong though because when I tried to install the generated raster_comments it complained that

function st_mapalgebrafct(raster, text, regprocedure, text[]) Does not exist.

Can you verify that one whether your documentation is wrong or the definition just got missed.

comment:21 Changed 5 years ago by robe

I took out the st_mapalgebrafct(raster, text, regprocedure, text[]) proto from the documentation at r7989 since its preventing me from installing postgis as an extension since the extension has to complete as a single transaction and includes raster comments. Fill free to add back if you define the function.

comment:22 in reply to:  20 Changed 5 years ago by dzwarg

Owner: changed from pracine to dzwarg

Replying to robe:

I made some more corrections at r7988. There is still something wrong though because when I tried to install the generated raster_comments it complained that

function st_mapalgebrafct(raster, text, regprocedure, text[]) Does not exist.

Can you verify that one whether your documentation is wrong or the definition just got missed.

Turns out that I edited rtpostgis.sql, and not rtpostgis.sql.in.c. I'm fixing that immediately. Sorry for the inconvenience.

comment:23 Changed 5 years ago by dzwarg

Status: newassigned

So here's a documentation question: I created 8 different version of ST_MapAlgebraFct, with different permutations of arguments. They can be grouped into 4 variants with and 4 variants without the last argument (text[]). Does it therefore make sense to have 4 variants documented, with the last argument (text[]) marked as optional?

In addition, should the simplest variant be documented first, then the more explicit variants later?

comment:24 Changed 5 years ago by robe

David,

My general opinion -- all versions (minus the _ which to us means private), should be documented. The main reason for me is when I scan my list of functions -- I look for things that don't have comments and if I see those -- I assume its an oversight or a mistake.

If you see no need to document their existence then you should think twice about why you are creating them and if you should have some of those arguments as optional. If marking them optional is not an option, then I say get rid of them and force people to type in the extra stuff if they get to that level of minutae. I know Pierre likes to have his ala carte full of choices, but sometimes choice is too overwhelming and should be saved for a second course.

comment:25 Changed 5 years ago by dzwarg

Resolution: fixed
Status: assignedclosed

Completed in r8135

Note: See TracTickets for help on using tickets.