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

prioritize snapping points in AutoGrid #1831

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

dmarek-flex
Copy link
Contributor

@dmarek-flex dmarek-flex commented Jul 12, 2024

I found the snapping_points idea very useful, but also that the default value for the min_step was too restrictive. Here I propose some changes that give priority to the snapping points over the computed interval boundaries.

  1. I changed the behavior so that when dl_min is defined, it becomes the size used as the min_step. This is needed for Simulations with PECs since the calculated max_dl is much too large to be useful.
  2. If the snapping_point is close to an existing boundary it will replace it. The logic I used should still result in intervals that satisfy the min_step requirement though.

Copy link
Collaborator

@weiliangjin2021 weiliangjin2021 left a comment

Choose a reason for hiding this comment

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

This is great, thanks! Just add a test and CHANGELOG and good to merge.

@dmarek-flex dmarek-flex force-pushed the dmarek/prioritize_snapping_points branch from d6d34b0 to 8cddb60 Compare July 12, 2024 21:17
@dmarek-flex dmarek-flex force-pushed the dmarek/prioritize_snapping_points branch from 8cddb60 to 11c43d2 Compare July 19, 2024 14:51
@dmarek-flex dmarek-flex requested a review from tylerflex July 19, 2024 15:09
@dmarek-flex
Copy link
Contributor Author

This is great, thanks! Just add a test and CHANGELOG and good to merge.

Done

@dmarek-flex
Copy link
Contributor Author

@tylerflex @weiliangjin2021 Any last comments?

@dmarek-flex dmarek-flex merged commit 68407d2 into pre/2.8 Jul 24, 2024
16 checks passed
@dmarek-flex dmarek-flex deleted the dmarek/prioritize_snapping_points branch July 24, 2024 17:22
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.

3 participants