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

Move propertylayer from DiscreteSpace to Grid #2431

Open
quaquel opened this issue Oct 28, 2024 · 5 comments
Open

Move propertylayer from DiscreteSpace to Grid #2431

quaquel opened this issue Oct 28, 2024 · 5 comments

Comments

@quaquel
Copy link
Member

quaquel commented Oct 28, 2024

In #2319, support for propertylayers was added to the new style grids. This was done in the root space class: DiscreteSpace. However, property layers are meaningful only, as far as I can tell, for orthogonal grids and hexgrids. I don't see how they would meaningfully work for Network or VoroinoiGrid. If I am correct, then the relevant code should be moved from DiscreteSpace into Grid.

@EwoutH
Copy link
Member

EwoutH commented Oct 28, 2024

The distinction here needs to be made if PropertyLayers have their own (independent) coordinate system, or inherit it from a Grid or other object.

I’m nudging towards the former, so you can have (for example) a Network on top of a PropertyLayer. But the PropertyLayer should probably be slightly extended to allow that.

CC @wang-boyu

@wang-boyu
Copy link
Member

Thanks for tagging me here. Unfortunately I'm not very familiar with the new style grids, so my comments below may not apply.

if PropertyLayers have their own (independent) coordinate system

In Mesa-Geo a "PropertyLayer" or rather, a raster layer can have its own coordinate reference system (crs), although it will be converted to the GeoSpace's common crs once it's added into the GeoSpace. For Mesa, I'd imagine that all PropertyLayers will share the same default crs, in which the bottom left corner is the origin? Otherwise we'll need to provide ways to convert crs (and possibly resolution) and it's not very straightforward.

a Network on top of a PropertyLayer

I guess a more common way of doing this would be through node attributes...? But I could be wrong about this.

@quaquel
Copy link
Member Author

quaquel commented Oct 28, 2024

In Mesa-Geo a "PropertyLayer" or rather, a raster layer can have its own coordinate reference system (crs), although it will be converted to the GeoSpace's common crs once it's added into the GeoSpace. For Mesa, I'd imagine that all PropertyLayers will share the same default crs, in which the bottom left corner is the origin? Otherwise we'll need to provide ways to convert crs (and possibly resolution) and it's not very straightforward.

This would be my preferred implementation for property layers in mesa itself. As far as I know, conversions are not supported at the moment. It would, therefore, be best to explicitly enforce that property layers have the same reference system by having property layers "inherit" the width and height from their owning space.

We can always revisit that choice down the line and provide conversion systems, but at the moment, we are stuck between both. We don't enforce a common reference system explicitly but don't provide any conversion. In my view, that's the worst of both worlds.

I guess a more common way of doing this would be through node attributes...? But I could be wrong about this.

In old-style networks, yes. In new-style networks, nothing is stored in the underlying graph itself. Cells have a properties dict that I think was intended for this, but that is something that needs to be fleshed out a bit more before 3.1.

@EwoutH
Copy link
Member

EwoutH commented Oct 30, 2024

I largely agree that PropertyLayers should have its own coordinate reference. By default that can match the square grids if that's useful.

that property layers have the same reference system by having property layers "inherit" the width and height from their owning space.

We can make that an option, but we have to make sure PropertyLayers can stand on their own
without a space. Also they need to be able to be linked two more than one space, which all might have their own coordinate system. So I wouldn't say any space can "own" a PropertyLayer, but more that it's linked to it (very core OOP this discussion).

I see all our two-dimensional spaces as a big cake. You can have as many layers you like an any layer can interact with any other layer (basically the GIS model).

We can always revisit that choice down the line and provide conversion systems, but at the moment, we are stuck between both. We don't enforce a common reference system explicitly but don't provide any conversion. In my view, that's the worst of both worlds.

Agreed. Maybe start enforcing it (since that's simplest). The current (only) CRS PropertyLayers understand is the default NumPy array "CRS", in which left bottom is 0, so let's only allow the PropertyLayer for square grids that follow that convention for now.

Then we can extend from there. But once we're integrating CRS checking and conversion, we're basically integrating Mesa-geo into Mesa right? Which I think is a great idea, but a larger effort, and we have to check that we're not building the same thing twice (of course we can build an improved 2.0 version of an idea).

@quaquel
Copy link
Member Author

quaquel commented Oct 30, 2024

but we have to make sure PropertyLayers can stand on their own without a space.

Yes, but in my this implies that there is no space at all. Otherwise a property layer should be tied to a space.

I see all our two-dimensional spaces as a big cake. You can have as many layers you like an any layer can interact with any other layer (basically the GIS model).

I am not sure what you mean. In my view, model should be open to having multiple spaces, but these spaces would not be stacked in a GIS sense. This stacking, in my view, is with one space and one or more property layers tied to this one space where a space and associated property layers share the same coordinate system (for now). I conceptually have no idea what it would mean to have 2 stacked spaces, while having a single space with property layers makes perfect sense.

Agreed. Maybe start enforcing it (since that's simplest). The current (only) CRS PropertyLayers understand is the default NumPy array "CRS", in which left bottom is 0, so let's only allow the PropertyLayer for square grids that follow that convention for NWO .

And hexgrids, because they have the same coordinate system but only a different neighborhood function.

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

No branches or pull requests

3 participants