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

Remove assumption of temperature #2

Closed
CommonClimate opened this issue Jul 13, 2022 · 3 comments
Closed

Remove assumption of temperature #2

CommonClimate opened this issue Jul 13, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@CommonClimate
Copy link
Collaborator

In several places, the code assumes that users only want to reconstruct temperature, and that this variable is called 'tas' (e.g. in prep_graphem). This is an unnecessarily restrictive assumption, and may lead to bugs if people try to reconstruct any other field, or have other variable names.

Since cfr stands for "climate field reconstruction", I suggest calling the target variable "field" in graphem-related functions. It may also be good to check that the LMR part of the code does not assume too much either.

@CommonClimate CommonClimate added the enhancement New feature or request label Jul 13, 2022
@fzhu2e
Copy link
Owner

fzhu2e commented Jul 15, 2022

Thanks. This issue is fixed in the latest version that has been merged into main.

Closing now. Please reopen if needed.

@fzhu2e fzhu2e closed this as completed Jul 15, 2022
@fzhu2e
Copy link
Owner

fzhu2e commented Jul 15, 2022

OK, I realized that it's not just my code, but also the original code. Reopening...

@fzhu2e fzhu2e reopened this Jul 15, 2022
@CommonClimate
Copy link
Collaborator Author

Yes, the original GraphEM code also assumes all climate fields are temperature. I will try to change that in my updates (see issue #4)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants