Skip to content

Cdm overhaul#965

Open
asinghvi17 wants to merge 108 commits into
as/vectordatacubesfrom
cdm_overhaul
Open

Cdm overhaul#965
asinghvi17 wants to merge 108 commits into
as/vectordatacubesfrom
cdm_overhaul

Conversation

@asinghvi17

Copy link
Copy Markdown
Collaborator

No description provided.

@asinghvi17

Copy link
Copy Markdown
Collaborator Author

@rafaqz is this ready to merge into the geometry lookup branch from your end?

@rafaqz

rafaqz commented May 6, 2025

Copy link
Copy Markdown
Owner

No, heaps more happening. Probably we will merge the other way it's more like "fix all of CF" now

@asinghvi17

Copy link
Copy Markdown
Collaborator Author

Yea that works too

@rafaqz

rafaqz commented May 6, 2025

Copy link
Copy Markdown
Owner

35 CF examples now! - all loading but not all in the optimal way. Rotations are still missing, and I'm not sure how to handle these lookups with a center point and an explicit polygon boundary.

With a regular lookup At would select the center point, and Contains would select anything in the cell (we should have used Coveredby for that, need to DE-I9M-ize DD asap)

So for this it could do the same thing? and maybe for all geometry lookups? At gets the centroid/representative point, and Contains or whatever checks if the point is in the area.

(I'm thinking we might need a DE19M.jl or DE19MPredicates.jl package to hold all of these wrapper types, then we can use them everywhere, like Extents.predicate(Contains(), a, b) and we have the predicate separated from the context.)

@asinghvi17

asinghvi17 commented May 6, 2025

Copy link
Copy Markdown
Collaborator Author

At gets the centroid/representative point, and Contains or whatever checks if the point is in the area.

I don't know about that....it seems pretty useless, the representative point must be within the geometry at any rate. At least for At.

@asinghvi17

Copy link
Copy Markdown
Collaborator Author

DE19MPredicates.jl

That would be nice...but would conflict with GeometryOps operations when we add those. Plus Rasters has Touches which is not DE9IM touches...

@rafaqz

rafaqz commented May 6, 2025

Copy link
Copy Markdown
Owner

Yeah swapping DD Touches and Contains is where the idea can from. I don't think it will conflict, are there types in GO? We can just use them there too?

@rafaqz

rafaqz commented May 6, 2025

Copy link
Copy Markdown
Owner

Haha then we can select with geometries omg

@asinghvi17

Copy link
Copy Markdown
Collaborator Author

Haha then we can select with geometries omg

We already can ;)

are there types in GO

No but there will be. And they have to subtype GOCore.Operation. That's so that they fit in the whole declarative API, if not then it's not really feasible...

One thing we can do is that DE9IMPreds.jl depends on GOCore, and Extents, and actually fills in the Extents.predicate method for these predicates. But then DD depends on GOCore, not sure if we want that.

@rafaqz

rafaqz commented May 6, 2025

Copy link
Copy Markdown
Owner

it seems pretty useless

Maybe, but I'm not sure what else At would do, probably error.

@rafaqz

rafaqz commented May 6, 2025

Copy link
Copy Markdown
Owner

We already can ;)

But like, with all the predicates even on a regular raster

@rafaqz

rafaqz commented May 6, 2025

Copy link
Copy Markdown
Owner

No DE19M is a barebones thing that Extents.jl could depend on. I think GO should just depend on it like DD depends on IntervalSets even though its not a selector.

@asinghvi17

asinghvi17 commented May 6, 2025

Copy link
Copy Markdown
Collaborator Author

The problem there is again the names will clash

But I don't mind putting func defs there that we specify can only be overloaded by types that the defining package owns. Then we can put the generic GI definitions in Rasters, but allow Extents to overload for Extents.jl types only. (So no (::Extent, ::Any))` in Extents, for example.

My issue is more about the GO end than the DD end to be clear.

@asinghvi17

asinghvi17 commented May 6, 2025

Copy link
Copy Markdown
Collaborator Author

But like, with all the predicates even on a regular raster

what does that mean? If I say Intersects(geom), then it can only give me either the bbox or a mergedlookup raster, neither of which are particularly nice.

How do we specify that the raster should be masked? Maybe a new MaskingIndexSet type? But that breaks the implicit promise of dims2indices, that it will return something vaguely sensible.

@rafaqz

rafaqz commented May 6, 2025

Copy link
Copy Markdown
Owner

I don't get why we need both predicate objects, the idea was to use the same thing rather than clash...

In simplest form it's a parameterised wrapper struct with default value of nothing.

That can be used for a lot of things. It can have a keyword holding method too like Dimension in DD

@asinghvi17

Copy link
Copy Markdown
Collaborator Author

I think we're thinking of two different objects. I'm thinking of the GO Operations interface, i.e.

abstract type GeometricPredicate <: Operation end

struct Contains{Alg{M}, Exact, ...} <: GeometricPredicate end

nargs(::Contains) = 2
application_level(::Contains) = GO.TraitTarget{GI.AbstractGeometryTrait}()

function (::Contains)(::GI.PointTrait, p1, ::GI.PointTrait, p2)
end

or something like this


My understanding is that you are thinking of more of a DD Dimension or Selector like struct?

The issue that I have is that GO.Contains() != DE9IM.Contains(), which will inevitably lead to name clashes basically everywhere. So we should figure out how to mitigate that. Maybe we just say that predicates are not GO operations? Not sure what the best solution is here. But this is the issue in my mind.

I don't know how the Operation interface will turn out - maybe we can make it semi trait based. Or maybe we don't end up doing it at all? It's kind of tricky.

github-actions Bot and others added 16 commits February 25, 2026 17:53
…isting compat) (#1051)

Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org>
Bumps [actions/checkout](https://github.com/actions/checkout) from 5 to 6.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v5...v6)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… existing compat) (#1053)

Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org>
Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>
…compat) (#1015)

Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org>
Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>
* zonal_emptyval

* remove accidental coverage changes
* force floating point for aggregate mean

* also use add_sum and mul_prod to define init conditions

* Update src/methods/aggregate.jl

Co-authored-by: Tiem van der Deure <tiemvanderdeure@gmail.com>

* add tests

* use Base.agg_sum in all agg

* bugfix add_sum

* bugfix rasterize

* bugfix

---------

Co-authored-by: Tiem van der Deure <tiemvanderdeure@gmail.com>
Co-authored-by: Anshul Singhvi <anshulsinghvi@gmail.com>
* update DD to 0.30 and bugfix

* bugfix

* add order

---------

Co-authored-by: Tiem van der Deure <tiemvanderdeure@gmail.com>
Bumps [julia-actions/setup-julia](https://github.com/julia-actions/setup-julia) from 2 to 3.
- [Release notes](https://github.com/julia-actions/setup-julia/releases)
- [Commits](julia-actions/setup-julia@v2...v3)

---
updated-dependencies:
- dependency-name: julia-actions/setup-julia
  dependency-version: '3'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 5 to 6.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md)
- [Commits](codecov/codecov-action@v5...v6)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…d mappedlookup (#1029)

* replace index with lookup a bunch of places

* Update test/array.jl

Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>

* Update src/lookup.jl

Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>

* Update src/lookup.jl

Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>

* revert a silly change to ncdatasets tests

* parent lookup 1

Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>

* parent lookup 2

Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>

* parent lookup in plotrecipes

* fix a bug

* fix plotrecipes

* projectedindex -> projectedlookup

---------

Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>
* use fill! instead of .= for type stability

* fix disaggregate to file test

---------

Co-authored-by: Anshul Singhvi <anshulsinghvi@gmail.com>
Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>
Bumps [julia-actions/cache](https://github.com/julia-actions/cache) from 2 to 3.
- [Release notes](https://github.com/julia-actions/cache/releases)
- [Commits](julia-actions/cache@v2...v3)

---
updated-dependencies:
- dependency-name: julia-actions/cache
  dependency-version: '3'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…existing compat) (#1072)

Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org>
@asinghvi17

Copy link
Copy Markdown
Collaborator Author

This apparently points to my vectordatacubes branch right now. Should this PR be closed and we open a new one to point to main?

rafaqz and others added 13 commits May 26, 2026 10:41
* shift locus before disaggregating a lookup

* add test

* more proper fix

* test that extent is preserved

* remove stray show

Co-authored-by: Felix Cremer <felix.cremer@dlr.de>

* tweak tests

* add a comment to `ceil`

* handle aggregation of irregular lookups

* fix a method ambiguity

* fix another method ambiguity

* index => lookup

---------

Co-authored-by: Felix Cremer <felix.cremer@dlr.de>
Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>
* Fixed range index bug

* fix range type

* Apply suggestion from @rafaqz

* Apply suggestion from @rafaqz

* anchored range

* use anchored range in aggregate too

* comment

* test against GDAL

* test against GDAL with 1 ups tolerance

* Apply suggestion from @rafaqz

* Apply suggestion from @rafaqz

* use StableRange

* add view/dotview methods

* more bugfix

* remove redundant TwicePrecision wrapper

Co-authored-by: Tiem van der Deure <tiemvanderdeure@gmail.com>

* use StableRange more widely, and allow stop inputs like StepRangeLen

* broadcast methods for StableRange

---------

Co-authored-by: dnstudent <davide.nicoli@unimi.it>
Co-authored-by: Tiem van der Deure <tiemvanderdeure@gmail.com>
…isation (#1067)

* update DD to 0.30 and bugfix

* bugfix

* Set rasterize count missingval to notzero

* Use MockChunkedDiskArray instead of RechunkedDiskArray in test

* Fix tests for rasterization

---------

Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants