Ticket #860 (closed enhancement: fixed)

Opened 2 years ago

Last modified 19 months ago

[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

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

Change History

  Changed 2 years ago by dzwarg

  • status changed from new to assigned

Changed 2 years ago by dzwarg

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

  Changed 2 years ago by pracine

  • priority changed from medium to blocker

Changed 2 years ago by dzwarg

Added prototype SQL for ST_MapAlgebra that accepts user functions.

Changed 2 years ago by dzwarg

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

Changed 2 years ago by dzwarg

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

  Changed 22 months ago by pracine

  • owner changed from dzwarg to pracine
  • status changed from assigned to new

  Changed 22 months ago by pracine

  • status changed from new to assigned

  Changed 20 months ago by dzwarg

  • owner changed from pracine to dzwarg
  • status changed from assigned to new

  Changed 20 months ago by dzwarg

  • status changed from new to assigned

  Changed 20 months 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.

follow-up: ↓ 12   Changed 20 months 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?

  Changed 20 months 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.

  Changed 20 months 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.

follow-up: ↓ 13   Changed 20 months ago by robe

I lean more on side of no.

in reply to: ↑ 8   Changed 20 months 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.

in reply to: ↑ 11   Changed 20 months 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.

  Changed 19 months ago by dzwarg

  • keywords history added

  Changed 19 months ago by dzwarg

Implemented code in r7980. Onto docs...

  Changed 19 months ago by dzwarg

  • owner changed from dzwarg to pracine
  • status changed from assigned to new

Updated docs in r7984. Reassigning for review before closing.

follow-up: ↓ 18   Changed 19 months 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.

in reply to: ↑ 17   Changed 19 months 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.

  Changed 19 months 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.

follow-up: ↓ 22   Changed 19 months 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.

  Changed 19 months 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.

in reply to: ↑ 20   Changed 19 months 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.

  Changed 19 months ago by dzwarg

  • status changed from new to assigned

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?

  Changed 19 months 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.

  Changed 19 months ago by dzwarg

  • status changed from assigned to closed
  • resolution set to fixed

Completed in r8135

Note: See TracTickets for help on using tickets.