Skip to content

[Fix] Round points instead of flooring#66

Merged
lucas-diedrich merged 1 commit into
fix/downloadfrom
fix/coordinate-rounding
Jun 11, 2026
Merged

[Fix] Round points instead of flooring#66
lucas-diedrich merged 1 commit into
fix/downloadfrom
fix/coordinate-rounding

Conversation

@lucas-diedrich

@lucas-diedrich lucas-diedrich commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

LMD xml files should only contain integer values.

py-LMD maps floats to integers via np.floor. Especially if all coordinates are relatively small and for smalll numerical imprecisions, this can lead to numbers that are very far off from the original number.

Discussed with @sophiamaedler

Example

This PR implements strategy 3

func1 = lambda xi: int(np.floor(xi)) # current strategy 
func2 = int # python integer 
func3 = lambda xi: int(round(xi, 0)) # rounding 

# example coordinates
# note the -1e-14 which is practically 0
x = [-1, -0.9, -0.5, -0.1, -1e-14, 0, 0.1, 0.5, 0.9, 1, 1.1, 1.5]

# func1: [-1, -1, -1, -1, 0, 0, 0, 0, 1, 1, 1] # -1e-14 rounded to -1
# func2:  [-1, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1] # -0.9 rounded 0
# func3: [-1, -1, 0, 0, 0, 0, 0, 1, 1, 1, 2] # reasonable rounding results

Rounding + integer gives the closest values to the actual coordinates

@lucas-diedrich lucas-diedrich self-assigned this Jun 10, 2026
@lucas-diedrich lucas-diedrich requested a review from mschwoer June 10, 2026 07:44
@lucas-diedrich lucas-diedrich removed the enhancement New feature or request label Jun 10, 2026
@lucas-diedrich lucas-diedrich force-pushed the fix/coordinate-rounding branch from 4d26990 to 9ff9408 Compare June 10, 2026 08:03
@lucas-diedrich lucas-diedrich changed the base branch from main to fix/download June 10, 2026 08:04

@mschwoer mschwoer 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.

ROFL

Comment thread src/lmd/lib.py
id = i + 1
x = ET.SubElement(root, f"X_CalibrationPoint_{id}")
x.text = f"{np.floor(point[0]).astype(int)}"
x.text = f"{int(round(point[0], 0))}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

do we know why it was floor in the first place :-)

Comment thread src/lmd/lib.py

y = ET.SubElement(shape, f"Y_{id}")
y.text = f"{np.floor(point[1]).astype(int)}"
y.text = f"{int(round(point[1], 0))}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

note sure if this is a breaking change: API etc all stays the same, only internal precision changes ..

@lucas-diedrich lucas-diedrich added bug Something isn't working and removed breaking-change labels Jun 11, 2026
@lucas-diedrich lucas-diedrich merged commit e49f833 into fix/download Jun 11, 2026
4 checks passed
@lucas-diedrich lucas-diedrich deleted the fix/coordinate-rounding branch June 11, 2026 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants