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

[POC] Report #569

Open
9 tasks
adebardo opened this issue Aug 23, 2024 · 3 comments
Open
9 tasks

[POC] Report #569

adebardo opened this issue Aug 23, 2024 · 3 comments
Labels
[POC] Conception To review Tickets needs approval about it conception

Comments

@adebardo
Copy link

adebardo commented Aug 23, 2024

Context

Now that we've created a CLI with coregistration and terrain attribute calculation steps, we can begin generating a report. For this, we'll take inspiration from what's done in demcompare and the README of demcoreg.

Configuration

What the user chooses to save or display will be specified in the configuration. For the report, we will try to generate it without saving the data.

[outputs]
path = "path/to/file"
save_terrain_attributes = true
save_coreg_results = true 
plot_terrain_attributes = false

Code

  • Add parameter validation.

  • In the run for saving results:

    if cfg["attributes"]["save_results"]:
        terrain_attr.save(attr + ".tif")
  • For plotting, wait for the merge of the new coregistration features.

Report Creation

The report should include:

  • The reference DEM.
  • The DEM to be aligned.
  • In case of coregistration:
    • The elevation difference before/after.
    • The first and last iteration of Nuth & Kaab.
  • The terrain attributes requested by the user.

Code

We are using sphinx. This functions may help :

estimation

3d

@adebardo adebardo added the [POC] Conception stuck The issue conception is stopped label Aug 23, 2024
@adebardo adebardo added [POC] Conception To review Tickets needs approval about it conception and removed [POC] Conception stuck The issue conception is stopped labels Sep 5, 2024
@adebardo
Copy link
Author

adebardo commented Sep 5, 2024

This ticket is a proposal and is open for discussion.

@rhugonnet
Copy link
Member

Looks all good to me!

For the format: I really like the demcompare report template done with Jinja2.
For the content of the report: Adding the plots you list sounds perfect. For the method-specific plot (like the slopetan/dh plot with aspect of Nuth and Kääb you mention), we should be able to fairly easily generalize the plots that a given Coreg method produces since #530 that stores consistently all binning/fitting parameters, and same iteration metadata.
This way, there would be a fixed plot template (single method to implemnt) for a 1D bin/fit (like NuthKaab), or a 2D bin/fit (like Deramp) and this across all classes.

On the statistics, I agree on having less default columns as we mentioned in the call.
Also, we could consider adding some metrics of spatial correlation before/after coregistration. Those metrics are usually the ones that improve the most after a coregistration/correction compared to just the spread (NMAD/STD), and the functions to derive these spatial correlation metrics already exist in xDEM.
This is a topic we recently discussed for the pc_align (coregistration) functionality of the Ames Stereo Pipeline: NeoGeographyToolkit/StereoPipeline#423 (comment). Not a priority of course, but something we can move towards 🙂

@adebardo
Copy link
Author

adebardo commented Sep 6, 2024

Hi @rhugonnet,
demcompare do not use Jinja2, but sphinx, it was a debate on demcompare so I tried Jinja2 but I can do it with sphinx, that's not a problem for me.

It's great that we can generalize the plots !

About the statisitics, I think we will discuss how to show them after having them implementing for the CLI. But i keep your remarks in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[POC] Conception To review Tickets needs approval about it conception
Projects
None yet
Development

No branches or pull requests

2 participants