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

[New issue] Extend PointCloud to do halo exchange and adjoint halo exchange #94

Open
MarekWlasak opened this issue Sep 26, 2022 · 9 comments
Assignees

Comments

@MarekWlasak
Copy link

Description

This is a blocker for https://github.com/JCSDA-internal/oops/pull/1924

Requirements

None

Acceptance Criteria (Definition of Done)

Passes adjoint test for halo exchange using the PointCloud functionspace.

Dependencies

None

@MarekWlasak
Copy link
Author

@gibbsp - This is another issue that should be in a JCSDA epic

@climbfuji
Copy link

@MarekWlasak I assume this issue belongs into the authoritative repository at ECMWF? We are planning to delete this fork.

@MarekWlasak
Copy link
Author

@climbfuji - I hope not. If you do, then any JSCDA contributions with have to be done in their own personal github spaces. Note that ecmwf has a policy to only send pull requests from forks.

@MarekWlasak
Copy link
Author

@ytremolet - are you aware that @climbfuji is planning to delete this fork. What fork are we planning to use in the atlas code sprint?

@climbfuji
Copy link

@ytremolet - are you aware that @climbfuji is planning to delete this fork. What fork are we planning to use in the atlas code sprint?

Yes - see #95 (comment). I think it makes a lot more sense to use a temporary branch in a user fork for suggesting changes to the authoritative repository, but if people want to maintain a JCSDA fork for this purpose, then that's fine (it comes at a small administrative overhead but I think that is justifyable) - but let's at least remove all permanent branches and only allow temporary branches for staging PRs to the authoritative repository. The whole point is to avoid divergence by having persistent branches in our own space, when we are using the authoritative branch for all applications.

Note that we also propose to remove our eckit, fckit and ecbuild forks based on the same argumentation.

@ytremolet
Copy link

We don't want to maintain our own version of atlas, so we should not have long lived branches that do not get merged into the ECMWF repo. But it's true that forks can be useful for testing, for code sprints, for issues/discussions, and for issuing PRs to ECMWF. Since we are doing enough work with atlas I propose we keep this fork for now.
The fckit, eckit and ecbuild ones can be deleted since we are not doing much work in those. We can always create temporary forks and delete them when we are done if needed. The important point is that those forks should not be used in any releases or long lived branches/bundles.
Would that work for everybody?

@climbfuji
Copy link

That works for me, yes. I'll make sure that we do not keep any long-lived branches in this fork. Thanks.

@climbfuji
Copy link

There are 79! branches in this fork. There should be only one long-lived one (so that there can be a default), namely develop with is kept in sync with the authoritative develop and not modified. Everything else needs to be short-lived.

@climbfuji
Copy link

There are now 7 branches left, one of them being "develop" - this is a direct sync of the ECMWF develop branch, solely because we need a default branch. The branch is locked down, nobody can write to it except fetching updates from upstream. The other six branches are "active" branches that were updated less than two months ago. There is no branch protection for any of these.

Going forward, we'll visit this fork from time to time and delete branches that have gone stale, i.e. no update for more than two months.

Does that sound reasonable?

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

No branches or pull requests

3 participants