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.

Sign up for GitHub

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

Jump to bottom

Add 3 tutorials for developers #895

Merged
merged 3 commits into from
Sep 27, 2022
Merged

Add 3 tutorials for developers #895

merged 3 commits into from
Sep 27, 2022

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Sep 21, 2022

  • Getting Started
  • Understanding a task
  • Adding a new task

@xylar xylar self-assigned this Sep 21, 2022
@xylar
Copy link
Collaborator Author

xylar commented Sep 21, 2022

@akturner, @alicebarthel, @anirban89, @cbegeman, @darincomeau, @jjbenedict, @katsmith133, @milenaveneziani

Please let me know if you notice problems with any of the tutorials. This would be an easy place to comment and fix things.

@xylar xylar force-pushed the add_developer_task_tutorial branch from 0f760a0 to 896d0b3 Compare September 22, 2022 00:52
milenaveneziani
milenaveneziani approved these changes Sep 22, 2022
View reviewed changes
Copy link
Collaborator

@milenaveneziani milenaveneziani left a comment

Choose a reason for hiding this comment

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

This looks great @xylar.

@xylar xylar force-pushed the add_developer_task_tutorial branch from 896d0b3 to 66562cf Compare September 24, 2022 17:42
@darincomeau
Copy link
Contributor

@xylar for the 'Add task' tutorial, an addition I'd recommend is indicate where the task needs to be added so that mpas_analysis will actually pick up and run the new task. For the sea ice production task I'm adding now, I found that I needed to add it to mpas_analysis/sea_ice/__init__.py and mpas_analysis/__main__.py.

@alicebarthel
Copy link

@darincomeau Thanks, your comment was really helpful.

@xylar
Copy link
Collaborator Author

xylar commented Sep 26, 2022

@darincomeau, you're totally right, that's the missing step. I will add that.

@xylar
Copy link
Collaborator Author

xylar commented Sep 26, 2022

@darincomeau, I did add that, but I didn't update the temporary webpage on my github.io. Take a look at this section and let me know if it addresses what you were missing:
https://github.com/MPAS-Dev/MPAS-Analysis/blob/66562cfee92ea73314243367975d96c8a8ea38cd/docs/tutorials/dev_add_task.rst#7-adding-the-task

@darincomeau
Copy link
Contributor

@xylar that looks great - thanks!

@xylar
Copy link
Collaborator Author

xylar commented Sep 26, 2022

There are some code blocks not showing up in the preview but which look right when I build the documentation.

darincomeau
darincomeau approved these changes Sep 26, 2022
View reviewed changes
Copy link
Contributor

@darincomeau darincomeau left a comment

Choose a reason for hiding this comment

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

Greatly appreciate your effort in putting together these tutorials!

jjbenedict
jjbenedict approved these changes Sep 26, 2022
View reviewed changes
Copy link
Collaborator

@jjbenedict jjbenedict left a comment

Choose a reason for hiding this comment

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

I can only confirm that the getting_started section worked for me. I have not done much with the other sections.

cbegeman
cbegeman approved these changes Sep 26, 2022
View reviewed changes
Copy link
Collaborator

@cbegeman cbegeman left a comment

Choose a reason for hiding this comment

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

I noticed a few typos but overall the tutorials look great. Thanks!

docs/tutorials/dev_understand_a_task.rst Outdated Show resolved Hide resolved
docs/tutorials/dev_understand_a_task.rst Outdated Show resolved Hide resolved
docs/tutorials/dev_understand_a_task.rst Outdated Show resolved Hide resolved
docs/tutorials/dev_add_task.rst Outdated Show resolved Hide resolved
@xylar xylar force-pushed the add_developer_task_tutorial branch from e2a6a3d to b86d41f Compare September 26, 2022 22:45
@xylar xylar force-pushed the add_developer_task_tutorial branch from 8867cec to c7c996f Compare September 26, 2022 22:46
@xylar
Copy link
Collaborator Author

xylar commented Sep 26, 2022

@cbegeman, thanks for finding those typos! (I'm sure not the last.) They should be fixed now.

anirban89
anirban89 reviewed Sep 26, 2022
View reviewed changes
Copy link
Collaborator

@anirban89 anirban89 left a comment

Choose a reason for hiding this comment

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

Do you want to add the updated task here in the tutorial? or leave the tutorial as is?

docs/tutorials/dev_add_task.rst
return bsfVertex


def _compute_barotropic_streamfunction_cell(dsMesh, bsfVertex):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here do we want to add the remapping to comparison grid?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you are referring to the Subpolar North Atlantic, no, let's keep this simple.

docs/tutorials/dev_add_task.rst

* get rid of ``ref_year_climatology_task`` since I'm not computing anomalies.

* get rid of ``depth_range`` because I'm using only the full ocean column.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we add the depth range back in here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so. Let's keep this as simple as possible for now.

@xylar
Copy link
Collaborator Author

xylar commented Sep 26, 2022

@anirban89, if there are things I need to change for the barotropic streamfunciton example to make it work that would be great! As far as adding complexity, I think we should leave that out of the tutorial. It's already complicated enough without new projeciton grids or depth ranges.

@alicebarthel
Copy link

@xylar this looks great! I did not go through the last one in detail but read through it to use it as an example for my own task. Very helpful!

@xylar xylar merged commit a8b62e9 into MPAS-Dev:develop Sep 27, 2022
@xylar xylar deleted the add_developer_task_tutorial branch September 27, 2022 18:00
@xylar
Copy link
Collaborator Author

xylar commented Sep 27, 2022

Thank you everyone! Very much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alicebarthel alicebarthel alicebarthel approved these changes

@jjbenedict jjbenedict jjbenedict approved these changes

@cbegeman cbegeman cbegeman approved these changes

@milenaveneziani milenaveneziani milenaveneziani approved these changes

@anirban89 anirban89 anirban89 approved these changes

@akturner akturner akturner approved these changes

@darincomeau darincomeau darincomeau approved these changes

Assignees

@xylar xylar

Labels
documenation enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@xylar @darincomeau @alicebarthel @akturner @anirban89 @milenaveneziani @cbegeman @jjbenedict