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

Creating an idealised example #55

Merged
merged 20 commits into from
Sep 25, 2024
Merged

Creating an idealised example #55

merged 20 commits into from
Sep 25, 2024

Conversation

michaeldenes
Copy link
Member

@michaeldenes michaeldenes commented Sep 24, 2024

This pull-request is to include a new example notebook, using an idealised flow (the Bickley Jet) to show how plasticparcels can be used, without the need to download any datasets.

With #56 , this is to close #51

@codecov-commenter
Copy link

codecov-commenter commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.66%. Comparing base (a692124) to head (9e4b578).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #55   +/-   ##
=======================================
  Coverage   66.66%   66.66%           
=======================================
  Files           8        8           
  Lines         717      717           
=======================================
  Hits          478      478           
  Misses        239      239           
Flag Coverage Δ
unit-tests 66.66% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@erikvansebille
Copy link
Member

I can't comment on the ipynb file using GitHub tools, so here a list of (minor) comments:

  • In cell 4, why not simply skip the VectorField plots (and use a counter that increments after every valid Field has been plotted)?
  • Before cell 5, perhaps provide links to other examples where the settings dictionary can be loaded?
  • In cell 5, mention where the b/c constants come from? Some values seem arbitrary, otherwise?
  • Perhaps for a later PR, but could the setting of fieldset-constants from the settings-dict not be a method?
  • In cell 10 (and later), how would the user know it's kernels[-2] that needs replacing?
  • In cell 29, perhaps print the final depth range with fewer digits?

docs/examples/idealised_flow.py Outdated Show resolved Hide resolved
docs/examples/idealised_flow.py Outdated Show resolved Hide resolved
docs/examples/idealised_flow.py Outdated Show resolved Hide resolved
docs/examples/idealised_flow.py Outdated Show resolved Hide resolved
docs/examples/idealised_flow.py Outdated Show resolved Hide resolved
docs/examples/idealised_flow.py Outdated Show resolved Hide resolved
docs/examples/idealised_flow.py Outdated Show resolved Hide resolved
@michaeldenes
Copy link
Member Author

michaeldenes commented Sep 24, 2024

  • Before cell 5, perhaps provide links to other examples where the settings dictionary can be loaded?

I've made a link to the examples page, just using 'see other examples'. Or should we flesh this out more?

  • In cell 5, mention where the b/c constants come from? Some values seem arbitrary, otherwise?

I've added to the comment that they come from the Kooi et al, Lobelle et al, and Kaandorp et al papers, with dois.

  • Perhaps for a later PR, but could the setting of fieldset-constants from the settings-dict not be a method?

I'm not sure I understand this one just yet, but maybe there's some refactoring we can do, yep!

  • In cell 10 (and later), how would the user know it's kernels[-2] that needs replacing?

I've added a note for cell 10, but have left all the later ones as is. Or should we add the note to each?

  • In cell 29, perhaps print the final depth range with fewer digits?

I've made it 4dp now. Perhaps we also print the difference?

@erikvansebille
Copy link
Member

Thanks for incorporating these, @michaeldenes. Two more comments on the notebook

  1. The new example is not listed at https://plasticparcels--55.org.readthedocs.build/en/55/examples.html?
  2. The else statement in cell 4 can be removed (instead of commented out)?

@michaeldenes
Copy link
Member Author

Thanks for incorporating these, @michaeldenes. Two more comments on the notebook

  1. The new example is not listed at https://plasticparcels--55.org.readthedocs.build/en/55/examples.html?
  2. The else statement in cell 4 can be removed (instead of commented out)?

Thanks @erikvansebille, I've now made these two changes

@michaeldenes michaeldenes merged commit c7d2dce into main Sep 25, 2024
5 checks passed
@michaeldenes michaeldenes deleted the idealised_example branch September 25, 2024 07:37
@michaeldenes michaeldenes mentioned this pull request Sep 25, 2024
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.

examples
3 participants