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

GmAPD Updates #42

Merged
merged 3 commits into from
Oct 11, 2018
Merged

GmAPD Updates #42

merged 3 commits into from
Oct 11, 2018

Conversation

kdamkjer
Copy link
Contributor

@kdamkjer kdamkjer commented Nov 8, 2017

Draft enhancements to support GmAPD LiDAR. Addresses Issue #35.

Draft enhancements to support GmAPD LiDAR.
@hobu
Copy link
Contributor

hobu commented Jan 10, 2018

After looking over these changes, my only suggestion is that the "aggregate" bit flag not only correspond to GmAPD systems, and it could also just as easily apply to dense imagery situations. I don't think we need to say anything about algorithms or consensus models on the description of the flag.

Copy link
Member

@rapidlasso rapidlasso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest that where ever the current version states "This is often the case for data generated by GmAPD systems." we replace this overly specific terminology with a much more general wording that can be used to any point cloud that is derived by combining multiple observations from some sensor (multiple images, photon counting arrays, averaged SAR scatterings, multiple linear full waveform LiDAR measurements accumulated in a voxel grid, sonar echo composited, etc) in contrast to the direct range observation by an active sensing system that LAS was originally designed for.

Copy link
Member

@rapidlasso rapidlasso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest that where ever the current version states "This is often the case for data generated by GmAPD systems." we replace this overly specific terminology with a much more general wording that can be used to any point cloud that is derived by combining multiple observations from some sensor (multiple images, photon counting arrays, averaged SAR scatterings, multiple linear full waveform LiDAR measurements accumulated in a voxel grid, sonar echo composited, etc) in contrast to the direct range observation by an active sensing system that LAS was originally designed for.

@anayegandhi
Copy link

Agree with comments from hobu and rapidlasso. Evon - can you please make these changes and then we seek final approval. Thanks.

Updates to address review comments from @hobu and @rapidlasso.
@kdamkjer
Copy link
Contributor Author

kdamkjer commented Oct 9, 2018

I made the changes requested w.r.t. GmAPD LiDAR systems language. I agree that the aggregate bit should apply to more than just GmAPD systems (which was hopefully reflected in the original language), but I think it is also important to acknowledge that it is being added so that GmAPD and other aggregate point clouds can be accepted as valid formatted LAS data. With the introduction of FWF, SPL, and GmAPD, perhaps there is a case for providing examples of valid formatted files to act as "Gold Standards".

@rapidlasso
Copy link
Member

Looks like the main specification document has changed too much since that pull request was made and now needs manual tweaking to be merged. Maybe someone neutral should volunteer. Please keep the language system neutral.

It occurred to me that if any system needs "special mention" it should be the SPL system because their "single shot of many beamlets" could actually benefit from special consideration whereas GML is just another point cloud derived from multiple observations similar to SfM / dense-matching ... (-:

@kdamkjer
Copy link
Contributor Author

kdamkjer commented Oct 9, 2018

To be clear, there's nothing preventing a GmAPD system from employing beamlets nor is it a defining characteristic of SPL.

@esilvia
Copy link
Member

esilvia commented Oct 11, 2018

I should have time tomorrow morning to clear up the merge conflicts from the structural changes I've been doing on the draft. Thanks for your patience while we iron out the final kinks from the GitHub transition.

@esilvia esilvia changed the base branch from master to draft-R14 October 11, 2018 00:11
@esilvia esilvia changed the base branch from draft-R14 to issue-35 October 11, 2018 00:18
@esilvia esilvia merged commit b974f00 into ASPRSorg:issue-35 Oct 11, 2018
@esilvia
Copy link
Member

esilvia commented Oct 11, 2018

I managed to resolve the merge conflicts WRT the source code reorganization and pushed the updates to @kdamkjer's branch. With those corrections this pull request looks good to me.

I have a couple questions about the wording that came up during my review, but that won't stop this pull request. I refreshed the issue branch and we can continue the conversation about details on #35.

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.

5 participants