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

Restructure Repository #72

Merged
merged 10 commits into from
Sep 18, 2024
Merged

Restructure Repository #72

merged 10 commits into from
Sep 18, 2024

Conversation

jatkinson1000
Copy link
Member

@jatkinson1000 jatkinson1000 commented Sep 18, 2024

This should close #71 and #67

  • separate CAM code from the core YOG parameterisation - see Move CAM interface code and tests out of NN_module  #67
  • Avoid duplication of code or dual-maintained copies of files across the repository
    • Use references between directories
    • Move to CMake instead of Make to handle dependencies
  • Move tests to their own location
    • Provide the option for users to specify what tests are built

@jatkinson1000 jatkinson1000 linked an issue Sep 18, 2024 that may be closed by this pull request
@jatkinson1000
Copy link
Member Author

Note that I have also removed the SAM interface file that was under development (see 2b3adf3).

This is because it is now deprecated and is no longer being pursued.
If returning it can be recovered from git history.

@jatkinson1000
Copy link
Member Author

One question I have is should we perhaps rename "NN_module" to "YOG_convection"?

This would be much clearer.
If I recall NN_Module was only ever a placeholder until someone told Jim and I what the "neural net we extracted to a module" shoudl be called.

Copy link
Collaborator

@MarionBWeinzierl MarionBWeinzierl left a comment

Choose a reason for hiding this comment

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

We went through this together this morning and this makes sense. I have some very minor comments below, but otherwise it can be merged.

Regarding the question about the renaming, I agree that "YOG_convection" is a more meaningful name -- this change could be included in this PR

README.md Outdated Show resolved Hide resolved
tests/CMakeLists.txt Outdated Show resolved Hide resolved
tests/cmake/FindNetCDF.cmake Show resolved Hide resolved
@jatkinson1000
Copy link
Member Author

Hi @MarionBWeinzierl Made those changes as suggested.
I'll go ahead and merge shortly.

@jatkinson1000 jatkinson1000 merged commit 2c0c079 into main Sep 18, 2024
1 check passed
@jatkinson1000 jatkinson1000 deleted the restructure branch September 18, 2024 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restructure repository to be more logical and useable Move CAM interface code and tests out of NN_module
2 participants