exeter-creww / SFM_Precision

A workflow to create precision maps in python
GNU General Public License v3.0
3 stars 1 forks source link

Setting CRS to 1 m - might not be okay? #43

Closed AndrewCunliffe closed 4 years ago

AndrewCunliffe commented 4 years ago

Setting CRS to 1 m. Is this always valid? If the CoordinateSystem truly is arbitrary, why is it okay to set units to 1m? Couldn't we just check earlier in the script that a valid (ideally m based) CorrdinateSystem is set? Lines 124-128 if chunk.crs is None: crs = Metashape.CoordinateSystem('LOCAL_CS["Local CS",LOCAL_DATUM["Local Datum",0],UNIT["metre",1]]') chunk.crs = crs else: crs = chunk.crs

h-a-graham commented 4 years ago

This is needed in case the user is using internal coordinates. From what Mike has noted this how the default internal coordinates are defined...

AndrewCunliffe commented 4 years ago

Maybe I'm misunderstanding something, but for projects using the arbitrary internal CRS, doesn't the above code just set the units to meters? (But the resulting model and precision values are not really in meters, just units called 'meters'). If so, the only benefit of this is in allowing the uncertainty of a reconstruction to be estimated in relative units... which is a pretty niche case. Shouldn't we just direct users to adopt a real CRS?

h-a-graham commented 4 years ago

Where would that leave a project like Pia's for example?

AndrewCunliffe commented 4 years ago

For projects like Pia, couldn't units be set when scale bars are incorporated? Or failing that, before running the precision analysis?

I'm just not convinced that setting none to meter opaquely will always be the right way to handle this. Yards are still very popular in the US, even if they get converted to metric when written up for publications... happy to chat next week if you want?

h-a-graham commented 4 years ago

Yeah okay - I see what you mean. Off the top of my head I don't know how the scale bars work so we can dig into that and then force the user to have some kind of real coordinates...

AndrewCunliffe commented 4 years ago

After a quick chat with Pia, we think it might make sense to say in the Readme something like:

"These scripts assume a metre-based coordinate reference system is being used. If a local coordinate system is used, the units must be set to metres. Units can be set to metres by entering XXX into the console."

Then if we want belt and braces approach, include a check within the script that the units are in metres?

piabenaud commented 4 years ago

Yeah, so it's not about real coordinates, but a metre-based crs or local cs, and definitely setting them up before using the precision script. If they're using scale-bars or otherwise they should have imported them long before getting to the precision analysis.

AndrewCunliffe commented 4 years ago

So we're agreed!

In SfMReadme, add: "These modules assume the project is using a metre-based coordinate reference system. Using non-metre-based systems has not been tested and may produce unexpected results."_

In the SFM_precision.py, change to: if chunk.crs is None: print("WARNING: no coordinate reference system set. Please set a (preferably metre-based) coordinate reference system before running module.") exit()

h-a-graham commented 4 years ago

All sorted - Now merged