WISE-Developers / Project_issues

This handles incoming tickets like bugs and feature requests
GNU Affero General Public License v3.0
2 stars 0 forks source link

[TASK] Test if #43 (Critical Paths for Assets ) outputs are GIS compatible. #87

Open spydmobile opened 2 years ago

spydmobile commented 2 years ago

@RobBryce wants this one tested by @BadgerOnABike. original: #43

BadgerOnABike commented 2 years ago

In order to perform this testing, @RobBryce do you have a method by which you instantiate the critical path system that I can leverage for use on my system? I would like to regenerate the product for testing.

BadgerOnABike commented 2 years ago

From everything I can see this is functioning. The json files come in as WGS 84 (EPSG 4326) while other vector and raster data are NAD83 10-TM (EPSG 3402). Was there more you were looking for here @RobBryce?

The biggest oddity to me is it seems to be a JSON with nested spatial features rather than a geojson? This is a different issue though.

spydmobile commented 2 years ago

@RobBryce & @BadgerOnABike Um, those outputs using NAD83 10-TM (EPSG 3402) are non-standard. we should output all of our data in a universal projection like WGS 84 (EPSG 4326) Just a thought...

BadgerOnABike commented 2 years ago

The NAD83 would be conforming to the fuel and elevation they were built off of. But I'm not against universality.

spydmobile commented 2 years ago

yes @BadgerOnABike but Users may not even have access to landscape source data.... but they will have access to exported results that need to be quickly ingested into some GIS or Geoserver that may not already know 3402.... so ya, exports should conform to a broader standard not that of the source landscape data Im thinkin.... again just thoughts, not dictating here...

BadgerOnABike commented 2 years ago

Yea that's a good call, there isn't a GIS out there that won't know 3402 as its a part of spatial reference so that shouldn't be an issue. It's when we get into other weird projections or user defined projections that are non-conformant with EPSG standards. That and I want to say Proj6 pulls directly from spatial reference. For the sake of user sanity, WGS84 is a never go wrong projection.

spydmobile commented 2 years ago

Right you are, Even geoserver knows it now :) so ya not a huge issue then, I for one, like some consistency & flexibility, so maybe we should be able to set a default export projection in settings somewhere, to override defaults. not that we can change geojson, since geojson is by definition WGS 84 (EPSG 4326) (or should be).

RobBryce commented 2 years ago

You can either embed the geojson inside a larger json file which contains arrival times (which is still valid json), or export to a different file. But when embedding, I don't think I'm explicitly providing the projection details. I need to fix that.

RobBryce commented 2 years ago

Using a specific projection is another matter. All vector data including critical paths use the same code and assumptions, being the input grid file projection. @spydmobile - I agree with you, and should there be a separate item to force KML's, GeoJSON's (whether fire perimeters or critical paths) to their respective default projections regardless of the fuel grid?

spydmobile commented 2 years ago

@RobBryce when you output geojson using gdal (and you dont reproject first) I think it reprojects automatically, since output geojson in wgs 84 as its doing means it is reprojecting the 10tm data to wgs84 on the fly for export. Cool. All I am saying is that if you are exporting, using gdal, you can likely also reproject before you write. or am I missing something?

You can either embed the geojson inside a larger json file which contains arrival times (which is still valid json), or export to a different file. But when embedding, I don't think I'm explicitly providing the projection details. I need to fix that.

If you are outputing geojson in must be wgs 84 and if you are outputing plain json, then I have no idea how you are doing it but it probably needs work to either conform to geojson, or its not really GIS compaitble IMO. So a better approach might be to output the spatial data in geoJSON and the non-spatial non-attribute data in a seperate non-spatial json file?

BadgerOnABike commented 2 years ago

I guess that's where my head was at too, the output is really janky when its a wrapped file versus what I've seen from a geojson and it makes interpreting the data anywhere else a mess. I think we got lucky and QGIS can see it and allows us to use it but the attribute table is kind of a mess.

The asset path seems to be more typical (looks like it is geojson afterall), while the individual assets are very hard to parse.

RobBryce commented 2 years ago

I think Q has issues with more complex outputs (I think it only recognized the 1st critical path when I tested it), which is one of the reasons why I supported a separate/companion output to GeoJSON. I didn't expect many people to want to embed it, but once it's extracted you have other issues like same directory or relative paths, and we haven't even gone down the rabbit hole of just sending this data directly to GeoServer.

RobBryce commented 2 years ago

@spydmobile - GDAL abstracts a lot of the differences in the file formats (KML, JSON, SHP, etc.) but it still gives us some control. I think it'd be legit to quickly check on the projections they are exporting and adjust accordingly, so it's unambiguous and documented. And yes, we have some control over what the projection is. If we should abandon the embedding approach, let's work from that, since it's just disabling some options I already have. If somebody is writing their own parser, then there may be some value if they can assume or check which projection the data is in.

BadgerOnABike commented 2 years ago

ArcGIS 10.5 and ArcPro cannot see or import the json files at all. The data interoperability extension is required, we don't even have it as a federal agency. Though that's more of an "ESRI not giving users basic functionality" issue.

spydmobile commented 2 years ago

ok, so @RobBryce looks like this one needs work? can you confirm and provide a plan and an estimate of efforts please?

RobBryce commented 2 years ago

I'm not sure what you wanted quoted on here. I'm re-reading it and still not sure. 1.) make sure that projection data is included in the output files? 2.) do you want critical path data embedded in the asset json file, or not? 3.) right now critical path data is associated with an asset, not the other way around, since critical paths are considered an optional piece of information above simply arrival time. Flipping the file could also make for implications for sub-scenarios. 4.) removing embedding of critical path data would simply turn off an option that is already there. Let's chat.

BadgerOnABike commented 2 years ago

First and foremost, I'd like these JSONs to be GeoJSONs. Currently they are only geographic data but are formatted as a typical JSON, Q is the only thing that has been able to guess effectively enough to get the data in. I also tried to get the data into R and received this:

FROM_GeoJson("D:/Quarantine/Prom_BP3_Testing/Critical Paths/v6a/testgcasset1b2.json") Error in export_From_geojson(url_file_string, Flatten_Coords, Average_Coordinates, : The output json object is NULL! See if any of the input data objects is not a valid json data type!

1) From RFC 7946

RFC7946 - Section 4

  1. Coordinate Reference System The coordinate reference system for all GeoJSON coordinates is a geographic coordinate reference system, using the World Geodetic System 1984 (WGS 84) [WGS84] datum, with longitude and latitude units of decimal degrees. This is equivalent to the coordinate reference system identified by the Open Geospatial Consortium (OGC) URN urn:ogc:def:crs:OGC::CRS84. An OPTIONAL third-position element SHALL be the height in meters above or below the WGS 84 reference ellipsoid. In the absence of elevation values, applications sensitive to height or depth SHOULD interpret positions as being at local ground or sea level. Note: the use of alternative coordinate reference systems was specified in [GJ2008], but it has been removed from this version of the specification because the use of different coordinate reference systems -- especially in the manner specified in [GJ2008] -- has proven to have interoperability issues. In general, GeoJSON processing software is not expected to have access to coordinate reference system databases or to have network access to coordinate reference system transformation parameters. However, where all involved parties have a prior arrangement, alternative coordinate reference systems can be used without risk of data being misinterpreted.

So, this means we stick with WGS84 for all geojson outputs.

2) The whole purpose of the asset import is to understand the critical path to that asset so I think embedding with the asset data makes sense? Having that able to be turned off wouldn't be a bad thing, but I have no idea why someone would want to? (Per 4)

3) If we're keeping CP with the asset I don't think this applies.

We can book some time to chat this week if you want to discuss further. I'm free Tues/Weds/Thurs.

spydmobile commented 2 years ago

@RobBryce @BadgerOnABike @nealmcloughlin I want to see this come to fruition, I know we have invested money and code in this but it is not yet ready for primetime, what do we need to do to move forward?

spydmobile commented 1 year ago

From Exec: @RobBryce We would like to do away with any json where spatial data is involved. Instead, if spatial data is involved, it should output in geojson, and we should stick to the chosen standard, and additionally the cardinality of a polygon has a spec in the standard and answers that concern. Lastly everything you can do ina. json, you can do in a geojson, just differently, so it should be the norm, unless there is no spatial data at all in which case json if fine. Proceed with a proposal for how to auto-generate filenames of sub scenario and we will review it.

RobBryce commented 1 year ago

It seems we have 2 topics inside of this item now.

1.) Confirmation on projection being applied to outputs correctly. I believe that Brett has reviewed outputs and is happy that data is in the correct projection.

2.) Issue with auto-generation of outputs. When you are working with a scenario, you can specify outputs to any GeoJSON or SHP file which is good - no issue with the current solution. But with sub-scenario's there's an issue with auto-generation of names when GeoJSON data is not to be embedded inside of a JSON file. 2.a.) Any auto-generation of filenames for sub-scenarios will have a way to break it. It could lead to a filename that is too large, or somehow duplicated in an Outputs directory that already exists, or if you dropped exports into sub-directories, then you could have naming conflicts by directory or filename, too, etc. This is similar to my comment in #132 where we cannot anticipate interference with any external tool. 2.b.) Embedding GeoJSON inside of JSON is valid and easy to programmatically unpack inside a GIS. But direction here says we aren't doing that. 2.c.) The only way I see going ahead here that could avoid conflict (from the WISE perspective) is to provide a way for the client code to specify how to generate filenames for sub-scenario outputs. This would let the user provide a string that is parsed by WISE and replace wildcards or parameter locations with values (like sub-scenario index). However, this may be language-specific or OS specific, which is why I'm posting this here before proposing any solution. E.g. rather than specifying an output filename as "Outputs/scenario_output.geojson", the client code would have to specify "Outputs/scenario%d_subscenario%d_output.geojson", where WISE would replace %d with appropriate values.

spydmobile commented 1 year ago

@RobBryce Please provide a plan for resolving these issues, I am partial to your ideas in 2c. Also an estimate for the time required.

BadgerOnABike commented 1 year ago

Did anything ever come of this?

1) Yes we were in WGS84

2) I also agree with C as that will ensure these files are actually geojsons and system will interpret them as such. This is also an opportunity for geopackages as the scenario can be the package and the subscenarios can each be layers.

spydmobile commented 1 year ago

@BadgerOnABike its a good idea to mention whoever you are asking ;-) Rob uses email to track github, just sayin ;-)