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 calculation for Terrestrial Coupling Index (TCI) #364

Closed
22 tasks
DanielAdriaansen opened this issue Apr 1, 2024 · 11 comments · Fixed by #369
Closed
22 tasks

Add calculation for Terrestrial Coupling Index (TCI) #364

DanielAdriaansen opened this issue Apr 1, 2024 · 11 comments · Fixed by #369
Assignees
Labels
METcalcpy: General Statistics priority: high High Priority requestor: NCAR/RAL NCAR Research Applications Laboratory required: FOR DEVELOPMENT RELEASE Required to be completed in the development release for the assigned project type: new feature Make it do something new

Comments

@DanielAdriaansen
Copy link

Describe the New Feature

I am updating the METplus Wrappers use case here: https://metplus.readthedocs.io/en/develop/generated/model_applications/land_surface/PointStat_fcstCESM_obsFLUXNET2015_TCI.html#sphx-glr-generated-model-applications-land-surface-pointstat-fcstcesm-obsfluxnet2015-tci-py. That use case does Python embedding for both the forecast and observation data. For the observation data, it was previously using pre-computed TCI data but now it will calculate TCI from raw observation data. The TCI calculation should be shared for the forecast and observation data since it is mathematically the same.

Acceptance Testing

List input data types and sources.
Describe tests required for new functionality.

Time Estimate

Estimate the amount of work required here.
Issues should represent approximately 1 to 3 days of work.

Sub-Issues

Consider breaking the new feature down into sub-issues.

  • Add a checkbox for each sub-issue here.

Relevant Deadlines

List relevant project deadlines here or state NONE.

Funding Source

Define the source of funding and account keys here or state NONE.

Define the Metadata

Assignee

  • Select engineer(s) or no engineer required
  • Select scientist(s) or no scientist required

Labels

  • Select component(s)
  • Select priority
  • Select requestor(s)

Projects and Milestone

  • Select Repository and/or Organization level Project(s) or add alert: NEED CYCLE ASSIGNMENT label
  • Select Milestone as the next official version or Future Versions

Define Related Issue(s)

Consider the impact to the other METplus components.

New Feature Checklist

See the METplus Workflow for details.

  • Complete the issue definition above, including the Time Estimate and Funding source.
  • Fork this repository or create a branch of develop.
    Branch name: feature_<Issue Number>_<Description>
  • Complete the development and test your changes.
  • Add/update log messages for easier debugging.
  • Add/update unit tests.
  • Add/update documentation.
  • Add any new Python packages to the METplus Components Python Requirements table.
  • Push local changes to GitHub.
  • Submit a pull request to merge into develop.
    Pull request: feature <Issue Number> <Description>
  • Define the pull request metadata, as permissions allow.
    Select: Reviewer(s) and Development issues
    Select: Repository level development cycle Project for the next official release
    Select: Milestone as the next official version
  • Iterate until the reviewer(s) accept and merge your changes.
  • Delete your fork or branch.
  • Close this issue.
@DanielAdriaansen DanielAdriaansen added type: new feature Make it do something new alert: NEED ACCOUNT KEY Need to assign an account key to this issue alert: NEED MORE DEFINITION Not yet actionable, additional definition required alert: NEED CYCLE ASSIGNMENT Need to assign to a release development cycle labels Apr 1, 2024
@DanielAdriaansen
Copy link
Author

I am not sure if this is "contributed" or a "diagnostic":
https://metcalcpy.readthedocs.io/en/latest/Contributors_Guide/index.html#organization-of-code-in-the-github-repository

Thoughts?

@DanielAdriaansen DanielAdriaansen removed the alert: NEED CYCLE ASSIGNMENT Need to assign to a release development cycle label Apr 1, 2024
@DanielAdriaansen
Copy link
Author

I am not sure if this is "contributed" or a "diagnostic": https://metcalcpy.readthedocs.io/en/latest/Contributors_Guide/index.html#organization-of-code-in-the-github-repository

Thoughts?

I think I could put this under contributed/land_sfc to be consistent with the METplus use case model_applications category: https://metplus.readthedocs.io/en/latest/generated/model_applications/index.html#land-surface?

Or, I could see a diagnostics/land_sfc directory, but I see the diagnostics directory is empty so maybe this is not the way to go yet.

@bikegeek
Copy link
Collaborator

bikegeek commented Apr 2, 2024 via email

@DanielAdriaansen
Copy link
Author

The contributed directory was created for outside collaborators, whose code didn't necessarily conform with our coding structure.

OK this helps me distinguish it a lot! The code I am adding is very small (30 lines?) and I have the freedom to write it in whatever standards we choose so I think ultimately this will land in diagnostics/land_surface. I am double checking with some other folks on the project. Thank you @bikegeek!

@bikegeek
Copy link
Collaborator

bikegeek commented Apr 2, 2024 via email

@DanielAdriaansen
Copy link
Author

@bikegeek one question I have is about structure under diagnostics. One option is to have a land_surface.py file, the other is to have diagnostics/land_surface/land_surface_diagnostic_name.py structure. Can you advise?

@bikegeek
Copy link
Collaborator

bikegeek commented Apr 2, 2024 via email

@DanielAdriaansen
Copy link
Author

DanielAdriaansen commented Apr 2, 2024

OK, then if I had this structure:
diagnostics/land_surface/terrestrial_coupling_index.py

and inside terrestrial_coupling_index.py there is a function called calc_tci(), can you help me formulate the import statement in my Python script?

Would it be:

from metcalcpy.diagnostics import land_surface as land_sfc
tci = land_sfc.calc_tci()

I don't think that's right, I think that's if it was diagnostics/land_surface.py. The extra directory confuses me for some reason but I am probably just forgetting how it works!

@bikegeek
Copy link
Collaborator

bikegeek commented Apr 2, 2024 via email

@anewman89
Copy link

@bikegeek @DanielAdriaansen This issue should use CLASP - 7790111 for charging.

@JohnHalleyGotway JohnHalleyGotway removed the alert: NEED ACCOUNT KEY Need to assign an account key to this issue label Apr 10, 2024
@DanielAdriaansen DanielAdriaansen linked a pull request Apr 11, 2024 that will close this issue
14 tasks
@DanielAdriaansen DanielAdriaansen removed the alert: NEED MORE DEFINITION Not yet actionable, additional definition required label Apr 12, 2024
@DanielAdriaansen
Copy link
Author

Closed via #369

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
METcalcpy: General Statistics priority: high High Priority requestor: NCAR/RAL NCAR Research Applications Laboratory required: FOR DEVELOPMENT RELEASE Required to be completed in the development release for the assigned project type: new feature Make it do something new
Projects
Status: 🏁 Done
Development

Successfully merging a pull request may close this issue.

5 participants