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

Math on all values throws error #537

Open
m-mohr opened this issue Feb 13, 2024 · 5 comments
Open

Math on all values throws error #537

m-mohr opened this issue Feb 13, 2024 · 5 comments

Comments

@m-mohr
Copy link
Member

m-mohr commented Feb 13, 2024

I'm trying to run

data = connection.load_collection(
  "ERA5",
  spatial_extent = None,
  temporal_extent = ["2000-01-01", "2000-01-02"],
  bands = ["u10", "v10"]
)
data = data ** 2.01 # also tried data.power(2.01)

I'd expect that it computes the power for all vales in the data cube (i.e. apply(process = power)).
Unfortunately I get:

Must be in band math mode already

@m-mohr m-mohr added the bug label Feb 13, 2024
@m-mohr
Copy link
Member Author

m-mohr commented Feb 13, 2024

Complete example that I ended up with:

from openeo.processes import sqrt 

data = connection.load_collection(
  "ERA5",
  spatial_extent = None,
  temporal_extent = ["2000-01-01", "2000-01-02"],
  bands = ["u10", "v10"]
)
u10 = data.band('u10') ** 2.01
v10 = data.band('v10') ** 2.01
data = u10 * v10
data = data.apply(process = lambda x: sqrt(x))
result = data.save_result("PNG")

What I'd like it to look like (or similar):

from openeo.processes import sqrt 

data = connection.load_collection(
  "AGERA5",
  spatial_extent = None,
  temporal_extent = ["2000-01-01", "2000-01-02"],
  bands = ["u10", "v10"]
)
data = data ** 2.01
u10 = data.band('u10')
v10 = data.band('v10')
data = sqrt(u10 * v10)
result = data.save_result("PNG")

@m-mohr
Copy link
Member Author

m-mohr commented Feb 13, 2024

Also, why is there no sqrt function on the datacube class? :-)

@soxofaan
Copy link
Member

soxofaan commented Feb 19, 2024

data ** 2.01

indeed, ** is not supported in non-band-math mode.
Other operators like data * 4 or 3 / data do work (i.e. translate to apply call) , but ** is indeed a todo here:

  • support ** in DataCube "apply" mode

@soxofaan
Copy link
Member

from openeo.processes import sqrt 
...
data = data.apply(process = lambda x: sqrt(x))

This can be simplified to

# no import necessary
...
data = data.apply(process = lambda x: x.sqrt())

or even

# no import necessary
...
data = data.apply(process="sqrt")

@soxofaan
Copy link
Member

Also, why is there no sqrt function on the datacube class? :-)

General answer: because the sqrt process is defined on a number value not a data cube value, so it doesn't make sense to define sqrt as data cube method.
There are indeed some existing DataCube methods like logarithm and log2 that violate that rule, but these are mostly legacy things.
I'm in doubt if it would be a good idea to provide a DataCube method for each openeo process that works on floats like sqrt, abs, ceil, floor, int, sgn, absolute, ... That would mean a lot of new methods with short names. While it would make the end user code more compact, I'm not sure it will make it more understandable.

E.g. we currently already have a bit of mixed behaviour:

# "Band-math" mode
cube.band("B02") + 5  # -> translates to reduce_dimension(dimension=bands, ...)
# merge cube mode
cube1 + cube2 # -> translates to merge_cubes(cube1, cube2, overlap_resolver=add)
# "Apply" mode
cube + 5  # -> translates to apply( x -> x+5)

The distinction is not obvious for the untrained eye (and it also not very easy to explain without getting too technical), so I'd prefer to promote a code style that keeps things more explicit.

For example for the apply mode:

cube.apply(lambda x: x+5)

The decoupling between the data cube and the apply "callback" is more obvious here, keeping things tidier if the callback gets more complex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants