GlacioHack / xdem

Analysis of digital elevation models (DEMs)
https://xdem.readthedocs.io/
MIT License
135 stars 39 forks source link

[POC] cli step 3 add coreg nuth&kaab #554

Open adebardo opened 1 month ago

adebardo commented 1 month ago

Context

The goal of this ticket is to implement coregistration from the xdem run.

In the code

From the run in the init file, we now need to:

[inputs]
reference_elev = "path/to_example.tif"
to_be_aligned_elev = "path/to_example.tif"

[outputs]
path = "path/to/save/folder"

coregdem, , , = dem_coregistration(reference_elev, to_be_aligned_elev, config["outputs"]["path"])


# For the tests

- [ ] Retrieve test data and ground truth data
- [ ] Verify that the outputs matches the ground truth from [here](https://github.com/GlacioHack/xdem/blob/main/tests/test_coreg/test_affine.py#L228) in the file `tests/test_cli.py`
- [ ] add tests for new entry 

# For the documentation

- [ ] Update the quick start guide in the documentation

2d
duboise-cnes commented 1 month ago

remarks:

Maybe in two steps, a first basic way as here to make a first cli and another adding a more flexible code organization. Seems not so difficult to add a factory in coreg of xdem. Don't know the potential impact on an API for now.

My 2 cts, hoping it helps

adehecq commented 1 month ago

A few comments:

And to answer @duboise-cnes's questions:

rhugonnet commented 1 month ago

For listing coreg methods: I think it would be easy to maintain a manual list, as we don't add a new method very often (there are list already used in test_biascorr, test_coreg/test_base and soon in test_affine, we could make them consistent by adding an available dictionary in each coreg sub-module as @duboise-cnes mentions). From the list we could automatically parse the different arguments of each coregistration class.

Note: Regarding the parameters, after the merge of PR https://github.com/GlacioHack/xdem/pull/530 (that I'm working on this week), all Affine coregistration methods will have exactly the same arguments across the different categories:

This is about to be documented (once https://github.com/GlacioHack/xdem/pull/502 is finalized, hopefully by the time you are all back from holidays :wink:).

adebardo commented 1 month ago

@duboise-cnes thanks, i think we could talk about the coregistration architecture in a meeting @adehecq Thanks for the tip. For the fit method, I would prefer to discuss the type of outputs you want and how we want to save things. @rhugonnet Great news for the parameters :)

I plan to revisit this matter after the 21/08/2024 meeting.

adehecq commented 1 week ago

I plan to revisit this matter after the 21/08/2024 meeting.

Is there anything new to discuss here?

adebardo commented 6 days ago

I plan to revisit this matter after the 21/08/2024 meeting.

Is there anything new to discuss here?

I already changed the ticket regarding the meeting yes, it's okay now