Skip to content

Preserve global Z offsets and support config groups in get_ordered_datasets#15

Open
msayr wants to merge 1 commit into
z_alignfrom
codex/investigate-lateral-jump-in-z-alignment
Open

Preserve global Z offsets and support config groups in get_ordered_datasets#15
msayr wants to merge 1 commit into
z_alignfrom
codex/investigate-lateral-jump-in-z-alignment

Conversation

@msayr

@msayr msayr commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Ensure dataset ordering handles nested config groups and avoids double-counting Z offsets when upstream prep already produced global Z coordinates.
  • Prevent incorrect stacking where a later group's offsets were blindly shifted by a previous group's length, causing duplicated Z displacement.

Description

  • Accept nested config_paths groups and collect datasets per group before computing offsets so groups can be treated atomically.
  • Compute group_offsets, group_z_start, and group_z_end arrays and detect whether a group already carries global Z offsets; preserve them when they overlap the current global cursor, otherwise shift the group's Z offsets by previous_offset.
  • Append dataset stores in group order, skip empty groups, and compute previous_offset as the maximal adjusted Z end across the group to correctly position subsequent groups.
  • Convert offsets to mutable lists and add informational logging about preserved global offsets.

Testing

  • Ran the project unit test suite with pytest and the tests completed successfully.

Codex Task

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 14538825c4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread emalign/align_z/utils.py
group_is_already_global = (
previous_offset > 0 and (
group_z_start.min() >= previous_offset or
group_z_end.max() > previous_offset

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Stop treating long local groups as global

When a later config group uses local Z offsets starting at 0 but is longer than the previous group, group_z_end.max() > previous_offset is true, so the group is preserved instead of shifted after the prior stack. For example, two XY configs with local ranges [0, 100) and [0, 200) leave the second group at Z 0 because 200 > 100, causing it to sort before/overlap the first group and corrupting the Z alignment plan that previously concatenated such configs.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant