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

Add grid support to MultipleDistantMeasure #274

Merged
merged 6 commits into from Oct 26, 2022

Conversation

leroyvn
Copy link
Member

@leroyvn leroyvn commented Oct 20, 2022

Description

This PR adds support for gridded viewing angle layouts to MultipleDistantMeasure. Changes are as follows:

  • MultipleDistantMeasure is reworked. The hplane and directions fields are replaced with a generic direction_layout field which holds a Layout instance. This field is dict-constructible, as always (I didn't use a Dessine-moi factory for this, it would be overkill IMO).
  • Layout defines an interface to generate a sequence of viewing directions and angles. This has several advantages:
    • Adding a new implementation is very simple.
    • The layout type can be used for specific operations (this is however no longer needed, see below!).
    • There are currently 5 layouts:
      • Directions: explicit specification of viewing directions as vectors.
      • Angles: explicit specification of viewing directions as angle pairs.
      • Hemisphere plane cut: as before, negative zenith values work naturally and no longer require fiddling with the output data during post-processing.
      • Azimuthal ring: as before.
      • Grid: directions are generated by making the Cartesian product of zenith and azimuth vectors.
  • The MultipleDistantMeasure.from_viewing_angles() constructor is deprecated and replaced by specialised constructors, all accessible directly or from the dict API using the construct key.
  • Post-processing is simpler: the assembly step no longer requires special treatment for the hplane layout.
  • A helper function is defined to help users grid against (VZA, VAA) the output of a gridded mdistant measure. For an example, see the unstack_gridded_helper.ipynb file in that gist.

This PR also contains small fixes and changes:

  • Azimuth normalisation now also snaps close-to-zero negative values to 0 to avoid irrelevant offsetting of inaccurately positioned values.
  • Tests are modified to use the numpy.allclose() function the way it is supposed to be, i.e. with the first argument as the reference.

To do

  • Code
    • Decide whether to remove or deprecate from_viewing_angles()
    • Add gridding helper function (see this gist for the basic principle)
    • Check if test coverage is sufficient
  • Docs
    • Add docstrings
    • Add Layout API docs
    • Update tutorials
    • Add measure guide explaining how all this works (other PR maybe?)
    • Add measure tutorial (other PR maybe?)
    • Add changelog entry

Checklist

  • The code follows the relevant coding guidelines
  • The code generates no new warnings
  • The code is appropriately documented
  • The code is tested to prove its function
  • The feature branch is rebased on the current state of the main branch
  • I updated the change log if relevant
  • I give permission that the Eradiate project may redistribute my contributions under the terms of its license

@leroyvn leroyvn marked this pull request as draft October 21, 2022 13:44
@leroyvn leroyvn force-pushed the refactor_mdistant branch 2 times, most recently from 588695a to 54078d7 Compare October 25, 2022 16:25
@leroyvn leroyvn marked this pull request as ready for review October 25, 2022 16:25
@leroyvn
Copy link
Member Author

leroyvn commented Oct 25, 2022

I think this is ready to ship. I decided to postpone the tutorials, it's a lot of work and I have other priorities. The usage of the new construction API is however almost the same as the previous (the new Layout abstraction layer is entirely hidden from the user and it's not even part of the public API). The gist I added to the description demonstrates how to use the xarray post-processing helper.

@leroyvn leroyvn merged commit 8705733 into eradiate:main Oct 26, 2022
@leroyvn leroyvn deleted the refactor_mdistant branch October 26, 2022 14:10
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.

None yet

2 participants