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

Devel/refactor #59

Merged
merged 39 commits into from
Jan 29, 2023
Merged

Devel/refactor #59

merged 39 commits into from
Jan 29, 2023

Conversation

oloapinivad
Copy link
Owner

This is a simple reorganization of the ecmean library and the relative functions. it is cleaner although now all the different functions are scattered around.

@oloapinivad
Copy link
Owner Author

This closes #51

@oloapinivad
Copy link
Owner Author

Last commits introduce a new way of parsing, closes #61

@oloapinivad
Copy link
Owner Author

Last commit has made a revolution, creating an ecmean package to be installed. this means that now we have to

  1. create the conda env with mamba env create -f environment.yml
  2. activate the environment conda activate ECmean4
  3. install the package pip install -e .

With the current version, we can even run global_mean from command line!

@oloapinivad oloapinivad mentioned this pull request Jan 23, 2023
@oloapinivad
Copy link
Owner Author

This closes #63, #64, #66

@oloapinivad
Copy link
Owner Author

oloapinivad commented Jan 24, 2023

This closes partially #65

@oloapinivad
Copy link
Owner Author

I found a small error in the way the cmor mask were handled (% instead of fraction) which is now solved. I also tried to introduce the oceanic mask. Now testing the computational performance.

@oloapinivad
Copy link
Owner Author

Test on levante suggest that there still a bit of confusion on two aspects

  1. The mask unit: we cannot use the one that is shipped within the file, often absent or badly formulated. The solution of dividing by 100 the cmor is the only one that look safe
  2. The recognition of regular/curvilinear/unstructured grid is everything but failsafe. However, since we plan to roll to CDO based interpolation, I will wait for this

One reflection regards the components. Each of them require a lot of manual handling. I am wondering if it will be a good idea to introduce a defaultatm and defaultoce component in line with CMOR, and then use exceptions from this

@oloapinivad
Copy link
Owner Author

@jhardenberg please let me know if you have time to test this. I would like to close and proceed this week.

@oloapinivad
Copy link
Owner Author

Introducing coverage (around 90%) to be published on coveralls.io and some more elaborated test for command line arguments. Merging soon.

@oloapinivad oloapinivad merged commit 4564b2b into main Jan 29, 2023
@oloapinivad oloapinivad deleted the devel/refactor branch March 7, 2023 06:03
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.

None yet

1 participant