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

Channel mobility metrics #28

Merged
merged 10 commits into from
Oct 19, 2020

Conversation

elbeejay
Copy link
Contributor

@elbeejay elbeejay commented Oct 4, 2020

This would go after #27.

Summary

This PR provides at least an initial take at adding channel mobility metrics to the deltametrics framework. Nothing fancy was done, so we can have a discussion about tying these functions in more tightly to deltametrics objects (right now they are much more 'standalone' than other modules we have). But the 4 metrics added are the following:

  1. Dry fraction decay per Cazanacli et al 2002

  2. Channel Planform Overlap per Wickert et al 2013

  3. Reworking Fraction per Wickert et al 2013

  4. Channel Abandonment per Liang et al 2016

A demo of the current functionality is available here. This PR does not include any substantial documentation beyond linking up the API reference, but in the future the plan would be to create documentation resembling that colab notebook.

Details

More specifically, this PR adds the following:

  • Allows for flexible type inputs to the mobility metrics
  • Functions to compute the above mobility metrics
  • Function for curve fitting linear, harmonic, and exponential functions
  • Unit tests the methods against the simple 'demo' test in addition to standard error/exception checking
  • Updates api reference to document these functions

This PR does not do the following:

  • Add an example of using these metrics to the documentation
  • Provide these functions as any kind of deltametrics class or object
  • Create a new 'base' class of any kind
  • Implement cool function names (I don't know that calc_"metric name here" is the nicest way to name a function)
  • Add methods to create more general (geometric) base masks, this could be something that is added to the masking functions later (e.g. a method to create binary mask of a radial region).
  • Manage the time dimension (these methods use indices from the arrays for setting basevalues and time_windows), in the future we could build-in the time 'units' via xarray

So like I said, I'm very flexible on changing the structure of these functions to better mesh with the deltametrics API, it was just not obvious to me what the best way of doing that would be. Part of my hesitation about writing a complex API / additional documentation is just due to the uncertainty around the merge order (will this go before or after #23 which breaks some bits of the API, either way I think the conflicts can be resolved without too much fuss). In conclusion, despite the shortcomings of this PR, I think it is still a step forward.

@codecov-commenter
Copy link

codecov-commenter commented Oct 4, 2020

Codecov Report

Merging #28 into develop will increase coverage by 1.23%.
The diff coverage is 98.29%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #28      +/-   ##
===========================================
+ Coverage    87.25%   88.48%   +1.23%     
===========================================
  Files           10       11       +1     
  Lines         1106     1251     +145     
===========================================
+ Hits           965     1107     +142     
- Misses         141      144       +3     
Impacted Files Coverage Δ
deltametrics/mask.py 95.66% <97.22%> (-0.29%) ⬇️
deltametrics/mobility.py 98.57% <98.57%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef0fe0f...4da41d4. Read the comment docs.

Copy link
Contributor

@amoodie amoodie left a comment

Choose a reason for hiding this comment

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

Hi @elbeejay

This is really great, thanks for contributing it! The notebook is a cool way to show it off, too.

I am wondering about the name of the module though (mobility). I'm not objecting, just trying to think about other metrics we will add. Probably a handful for shoreline mobility. Channel network metrics. Shoreline position/shape metrics. I think these all fall into something like planform metrics but we already have the Planform object housed in plan.py, and I'd like to keep those separate. I supposed mobility is probably broad enough it could encompass enough functions to make a complete module, but where do the shoreline_position metrics go then? Just thinking out loud here.

Please open issues for the items in your "does not do" checklist, where appropriate.

deltametrics/mobility.py Outdated Show resolved Hide resolved
deltametrics/mobility.py Outdated Show resolved Hide resolved
docs/source/reference/mobility/index.rst Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Oct 19, 2020

Codecov Report

Merging #28 into develop will increase coverage by 1.23%.
The diff coverage is 98.29%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #28      +/-   ##
===========================================
+ Coverage    87.25%   88.48%   +1.23%     
===========================================
  Files           10       11       +1     
  Lines         1106     1251     +145     
===========================================
+ Hits           965     1107     +142     
- Misses         141      144       +3     
Impacted Files Coverage Δ
deltametrics/mask.py 95.66% <97.22%> (-0.29%) ⬇️
deltametrics/mobility.py 98.30% <98.30%> (ø)
deltametrics/utils.py 60.00% <100.00%> (+18.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef0fe0f...07186f3. Read the comment docs.

@amoodie amoodie merged commit fe80c01 into sandpiper-toolchain:develop Oct 19, 2020
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.

4 participants