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

Fixed ordering of fixup_halos and halo_exhange.execute_adjoint in StructuredColumns.cc #223

Conversation

odlomax
Copy link
Contributor

@odlomax odlomax commented Sep 24, 2024

Whilst integrating util::pack_vector_fields into the SphericalVector interpolation method, I uncovered a bug in StructuredColumns. The methods fixup_halos and halo_exchange.execute_adjoint were called in the wrong order during a halo exchange adjoint. This PR fixes the bug.

I'd initially missed this by adding (and then forgetting) a workaround in the SphericalVector test.

@MarekWlasak this will likely be of interest to you.

@odlomax odlomax changed the title Fixed ordering of fixup_halos and halo_exhange in StructuredColumns.cc Fixed ordering of fixup_halos and halo_exhange.execute_adjoint in StructuredColumns.cc Sep 25, 2024
@odlomax
Copy link
Contributor Author

odlomax commented Oct 8, 2024

@wdeconinck Am I correct in thinking the test failure isn't related to these commits? Let me know if there's anything I can do.

@wdeconinck
Copy link
Member

I would like to see what @MarekWlasak has to say about this, otherwise it is good to merge for me.

@odlomax
Copy link
Contributor Author

odlomax commented Oct 10, 2024

@MarekWlasak, please could you bless or condemn this PR!

@MarekWlasak
Copy link
Contributor

@wdeconinck - This PR is blessed!

@wdeconinck wdeconinck merged commit 10cffa6 into ecmwf:develop Oct 11, 2024
159 of 163 checks passed
wdeconinck added a commit that referenced this pull request Nov 18, 2024
* release/0.40.0: (40 commits)
  Update changelog
  Version 0.40.0
  Add hicLaunchHostFunc for async host functions
  Add test for various use cases of StructuredInterpolation2D with limiter
  Implement StructureInterpolation2D for 3-dimensional fields with extra variables)
  Fix application of limiter for StructuredInterpolation2D
  Protect atlas_test_array_view_variant to eckit v 1.27.0
  Move example-fortran inside atlas_HAVE_DOCS (#235)
  Fix possible device-memory leak in WrappedDataStore (fixes #234)
  add device data on FieldSet
  Add test for fieldset device operators
  Add fieldset device operators
  Update fctest_field_gpu to test acc deviceptr
  Update test_field_acc to test acc deviceptr
  Add device_data Fortran access to Field
  Refactored spherical vector interpolation method. (#227)
  Refactored (un)pack_vector_fields. (#226)
  Integrate `pack_vector_fields` into `SphericalVector` Interpolation method. (#224)
  Fixed ordering of fixup_halos and halo_exhange in StructuredColumns.cc (#223)
  Tidied headers and error message. (#231)
  ...
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.

3 participants