-
Notifications
You must be signed in to change notification settings - Fork 44
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
Enable snapping_points in grid generation #1719
Conversation
53747ed
to
f5a2e3e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @weiliangjin2021 see my comments.
6478c09
to
d0d0e5b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @weiliangjin2021 ! Finally, the snapping points should replace the diagonally placed meshoverride boxes to handle edges. Regarding the open question about the threshold, from my understanding, it is currently determined by the largest allowable step for each structure interval. Adding two more parameters—one for the smallest allowable step and another for the enforced step at the point—might be useful. Perhaps these two parameters could be combined into one.
Not really. Along a direction, the simulation domain is divided into multiple intervals. Each interval has its own maximal allowed step size. The threshold value is half of the minimal of those steps sizes.
Could you explain a bit more? |
I see. I misunderstood it as the smallest allowed step. So currently, the minimum step is given by 'min_steps_per_wvl' and the largest time step is given by these intervals. Is this correct? |
Is there a typo in your sentence (how does time step enter here)? 'min_steps_per_wvl' determines the maximal allowed step size in each interval. |
Sorry it is a typo, ‘space’ step.
…On Wed, May 29, 2024 at 5:03 PM weiliangjin2021 ***@***.***> wrote:
I see. I misunderstood it as the smallest allowed step. So currently, the
minimum step is given by 'min_steps_per_wvl' and the largest time step is
given by these intervals. Is this correct?
Is there a typo in your sentence (how does time step enter here)?
'min_steps_per_wvl' determines the maximal allowed step size in each
interval.
—
Reply to this email directly, view it on GitHub
<#1719 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BGTANRPWAR6MNTL7OCCPIN3ZEY7ANAVCNFSM6AAAAABIIGEIN2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZYGI2TSNJWGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
d0d0e5b
to
155a1a8
Compare
Well, then I still don't get your question. I guess it should be clear that the threshold is |
155a1a8
to
392f4bb
Compare
I tested with several examples and understand the threshold. The parameter looks good to me! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, main comment from usability perspective is that in some cases you might want to set the point along e.g. one of the axes only, but I think it's fine, we don't need to complicate things so much for now.
392f4bb
to
8e2a835
Compare
8e2a835
to
5d3d186
Compare
TODO:
Feature Description
Enforce placing grid points at certain locations. This is helpful in improving simulation accuracy by enforcing grid points at certain geometry singularities. Around the snapping point, the maximal step size is unmodified.
Implementation:
parse_structures
. If any interval contains a snapping point, split the interval. The maximal grid step size will be the same at the two splitted intervals.np.amin(max_dl_list) * 0.5
, but it's an open question and worth more investigation.Example
Here is an example comparing the grids after the insertion of snapping points: