search

Riverscapes / gcd

Geomorphic Change Detection For Windows
http://gcd.riverscapes.xyz
GNU General Public License v3.0
25 stars 5 forks source link

Budget Segregation Results Change With Repeat Viewing #355

Closed philipbaileynar closed 5 years ago

philipbaileynar commented 5 years ago

This problem was reported by Will Conley on 5th Aug 2019. He reports that repeatedly opening the budget segregation results window alters the volumetric results displayed. See the sequences of screenshots below showing the same BS results in the same project being viewed in consecutive GCD sessions.

The problem appears to be somewhat erratic. It's suspected that a complete shutdown and reload of GCD might be needed to reproduce the problem. Also, it might require a change to the GCD project (which would cause a subsequent write of the GCD project file).

Note also that Will's project units are yards and cubic yards. This makes me believe that there is a simple error where volumetric units in the project file are assumed to be in the wrong units and are converted when reading or writing the project file.

This problem might not have been observed by other people because most users' data are in metres and do not require any conversion during read and write operations.

Note that Will has verified that the sequential changes to the volumetric values is also manifested in the project file XML.

unnamed-7 unnamed-2

unnamed-1

fluviotect commented 5 years ago

Some additional info:

philipbaileynar commented 5 years ago

I can confirm:

The easiest way to recreate the problem is:

  1. create a new project with non-metric units.
  2. Add two DEMs and perform a single DoD.
  3. Close the project.
  4. Open the project and view the DoD results.
  5. Edit the name of the DoD and click update (this causes a write of the entire GCD project).
  6. Close the GCD.
  7. Open the GCD and view the DoD results. The volumes will have changed.
philipbaileynar commented 5 years ago

I have found and fixed the problem in GetVolume.

The GCDAreaVolume class stores areas and volumes in a single object using unitless counts of cells and the sum of cell values. As this object is constructed the raw values (either read from the GCD project file during load, or directly from a recent DoD or BS) are converted to metric so as to guarantee consistency. Therefore the sum of cells is actually a metric value.

During retrieval (for display or writing the GCD project when a change occurs) the sum was being read and interpreted using the vertical units of the project. This was fine when the project was metric because everything was consistent. But for projects using imperial units there was a mismatch as the sum stored as a metric value was erroneously interpreted as metric.

The fix was one line of code. I also wrote a unit test to verify that it works and that there's no regression to the metric case. However, the change necessitated the change to the GetVolume() function signature which caused about 20-30 calls to this method to change slightly.

fluviotect commented 5 years ago

Cheers Philip!

On Sat, Aug 10, 2019 at 5:31 AM Philip Bailey notifications@github.com wrote:

I have found and fixed the problem in GetVolume https://github.com/Riverscapes/gcd/blob/master/GCDConsoleLib/GCD/GCDAreaVolume.cs#L107 .

The GCDAreaVolume class stores areas and volumes in a single object using unitless counts of cells and the sum of cell values. As this object is constructed the raw values (either read from the GCD project file during load, or directly from a recent DoD or BS) are converted to metric so as to guarantee consistency. Therefore the sum of cells is actually a metric value.

During retrieval (for display or writing the GCD project when a change occurs) the sum was being read and interpreted using the vertical units of the project. This was fine when the project was metric because everything was consistent. But for projects using imperial units there was a mismatch as the sum stored as a metric value was erroneously interpreted as metric.

The fix was one line of code. I also wrote a unit test https://github.com/Riverscapes/gcd/commit/364bc659c6386e43adb58335a0694747820a3bda#diff-4f2f79dd886270cf7b025614ba1f95f3 to verify that it works and that there's no regression to the metric case. However, the change necessitated the change to the GetVolume() function signature which caused about 20-30 calls to this method to change slightly.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Riverscapes/gcd/issues/355?email_source=notifications&email_token=AJ7FW2VVJCC26TDGKCTFQFTQDWSWHA5CNFSM4IKEFDOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD37JO7Y#issuecomment-520001407, or mute the thread https://github.com/notifications/unsubscribe-auth/AJ7FW2VNYRPB3ECKDMILLBTQDWSWHANCNFSM4IKEFDOA .

fluviotect commented 5 years ago