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

[Bug]: Potential bug in temporal.py module #695

Closed
zhangshixuan1987 opened this issue Sep 21, 2024 · 4 comments · Fixed by #696
Closed

[Bug]: Potential bug in temporal.py module #695

zhangshixuan1987 opened this issue Sep 21, 2024 · 4 comments · Fixed by #696
Assignees
Labels
type: bug Inconsistencies or issues which will cause an issue or problem for users or implementors.

Comments

@zhangshixuan1987
Copy link

What happened?

I recently used the PCMDI metrics package to process the diagnostics for the piControl simulations from the E3SM model. As the model time for the piControl starts from year 1, issues arises when the PCMDI calls the function _drop_incomplete_djf () in "xcdat/temporal.py", specifically, I found the error are pointed to line 1087:

incomplete_seasons = (f"{start_year}-01", f"{start_year}-02", f"{end_year}-12")

The problem to my understanding here is that the start_year and end_year in the piControl simulations (e.g. year 1 to year 50) are not 4-digit format, then the _drop_incomplete_djf () complains in my case that:

warning: failed for v3-SORRM 0051 no ISO-8601 or cftime-string-like match for string: 1-01

I think the function actually expect the "0001-01" rather than "1-01", which caused the problems for my case.

I think that it would be safer to adjust the 1087 in "xcdat/temporal.py" to

incomplete_seasons = (f"{start_year:04d}-01", f"{start_year:04d}-02", f"{end_year:04d}-12")
In this way, the 4-digt year will be guaranteed, and my test simulation can pass the above error for piControl simulations.

I reported this use case here to see if this would be an indicator for some code improvements.

What did you expect to happen? Are there are possible answers you came across?

No response

Minimal Complete Verifiable Example (MVCE)

No response

Relevant log output

No response

Anything else we need to know?

No response

Environment

The xCDAT is part of library for PCMDI metrics (https://github.com/PCMDI/pcmdi_metrics). In my case, xcdat version 0.7.1 was complied with the python 3.10 and used as the library for the mcmdi metrics.

@zhangshixuan1987 zhangshixuan1987 added the type: bug Inconsistencies or issues which will cause an issue or problem for users or implementors. label Sep 21, 2024
@tomvothecoder
Copy link
Collaborator

Hi @zhangshixuan1987, thank you for reporting this issue. @lee1043 opened up PR #696 to fix this. We plan on releasing a new version of xcdat (v0.7.2) in the next few weeks that should include this fix.

@lee1043
Copy link
Collaborator

lee1043 commented Sep 23, 2024

@zhangshixuan1987 I just merged #696 so it is in the main branch now. In the meantime if you like to have the updated functionality in your workflow as early adapter and beta tester, you can clone xcdat github repository to your local, activate your conda env, and manually install xcdat by "pip install ." from your local xcdat directory. This can expedite your test for piControl. Please let us know whether the updated code works okay for you.

@lee1043 lee1043 self-assigned this Sep 23, 2024
@zhangshixuan1987
Copy link
Author

zhangshixuan1987 commented Sep 23, 2024 via email

@zhangshixuan1987
Copy link
Author

@lee1043 @tomvothecoder : Thank you both for the quick response to my questions and proposed the fix. I will update my library accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Inconsistencies or issues which will cause an issue or problem for users or implementors.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants