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

Incorporate the Ftorch branch into the CAM-Interface branch #63

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

tztsai
Copy link
Collaborator

@tztsai tztsai commented Mar 19, 2024

Summary:

This branch aims to integrate Elliot's ftorch branch with Jack's cam-interface branch. The key changes involve resolving conflicts between test.f90 and nn_convection_flux.f90. To address these conflicts, test.f90 is being rewritten to align with the latest version of nn_convection_flux.f90 from Jack's branch.

Progress:

  • Resolve conflicts between test.f90 and nn_convection_flux.f90
  • Rewrite test.f90 based on Jack's nn_convection_flux.f90
  • Ensure project builds successfully with integrated changes
  • Add tests for new functionality in test.f90

Details:

The integration of Elliot's ftorch branch and Jack's cam-interface branch has been a challenging task due to conflicting changes in test.f90 and nn_convection_flux.f90. To resolve these conflicts, a thorough understanding of the intended functionality in nn_convection_flux.f90 is required, as this module is a dependency for test.f90.

The current approach is to rewrite test.f90 from scratch, ensuring it aligns with the latest version of nn_convection_flux.f90 from Jack's branch. This will involve implementing new tests to verify the correct behavior of the integrated code.

Once the conflicts are resolved and the project builds successfully, additional tests will be added to test.f90 to ensure the new functionality is thoroughly covered.

Review Notes:

Reviewers should focus on the changes made to test.f90 and its interactions with nn_convection_flux.f90. Particular attention should be given to the correctness of the new tests and their coverage of the intended functionality.

@tztsai tztsai changed the base branch from main to cam-interface March 19, 2024 11:06
@tztsai
Copy link
Collaborator Author

tztsai commented Mar 20, 2024

Currently there is a memory error double free or corruption (!prev) when running test.f90. I think it's because I adopted Elliot's version of test.f90 which contains new tests of the ftorch model and Jack's version of nn_convection_flux.f90 . I suspect there's some inconsistency between these two files as both of Elliot and Jack have introduced lots of changes separately.

@tztsai tztsai changed the title Ftorch 2 Incorporate the Ftorch branch into the CAM-Interface branch Mar 26, 2024
@tztsai tztsai self-assigned this Mar 26, 2024
@tztsai
Copy link
Collaborator Author

tztsai commented Mar 27, 2024

I used valgrind to debug the memory error and found that the segmentation fault occurs at

features(dim_counter+1:dim_counter+input_ver_dim) = real(q_i(i,1:input_ver_dim),4)

I commented out this line and its following line and now test.f90 can run successfully without raising any error.

@jatkinson1000 Are these lines supposed to be kept (so n_inputs has a wrong size), or should they be commented out since the rest of its block is already commented?

@tztsai tztsai marked this pull request as ready for review March 27, 2024 17:30
@tztsai tztsai requested a review from jatkinson1000 March 27, 2024 17:30
Base automatically changed from cam-interface to main August 23, 2024 12:18
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.

2 participants