-
Notifications
You must be signed in to change notification settings - Fork 637
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
Add Python support for Prisms #345
Conversation
python/typemap_utils.cpp
Outdated
if (!pyv3_to_v3(PyList_GetItem(py_vert_list, i), &v3)) { | ||
return 0; | ||
} | ||
vertices[i].x = v3.x; |
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.
Why not just vertices[i] = v3
?
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.
Done.
It means that it is discretized slightly differently than before. The real question is whether the discretization error has increased or is just different. A good way to test this might be:
If the prisms are much worse, then it would suggest a bug in the subpixel averaging of stevengj/libctl#13. |
It looks to me like the |
Did you look at output-epsilon to make sure that the structures look the same? e.g. maybe the prisms don't extend all the way to the edge of the cell like the blocks do? |
Here's a minimal C++ reproducer representing an inconsistency I found while stepping through the block and prism versions line by line. int main(int argc, char *argv[])
{
meep_geom::material_type mat = meep_geom::make_dielectric(12);
vector3 center = {0, 0, 0};
vector3 e1 = {1, 0, 0};
vector3 e2 = {0, 1, 0};
vector3 e3 = {0, 0, 1};
vector3 size = {1, 1, 0};
geometric_object block = make_block(mat, center, e1, e2, e3, size);
vector3 vertices[4] = {
{-0.5, 0.5, 0},
{0.5, 0.5, 0},
{0.5, -0.5, 0},
{-0.5, -0.5, 0}
};
vector3 axis = {0, 0, 1};
geometric_object prism = make_prism(mat, vertices, 4, 0, axis);
vector3 p = {-0.5, 0, 0};
vector3 nhat_block = normal_to_object(p, block);
vector3 nhat_prism = normal_to_object(p, prism);
double tolerance = 1.0e-6;
if (!vector3_nearly_equal(nhat_block, nhat_prism, tolerance)) {
printf("FAIL: Normals not equal\n");
printf(" Block: {%f, %f, %f}\n", nhat_block.x, nhat_block.y, nhat_block.z);
printf(" Prism: {%f, %f, %f}\n", nhat_prism.x, nhat_prism.y, nhat_prism.z);
}
return 0;
} |
The issue is that normal = unit_vector3(normal_to_fixed_object(vector3_minus(p, shiftby), *o));
if (normal.x == 0 && normal.y == 0 && normal.z == 0)
goto noavg; // couldn't get normal vector for this point, punt By contrast, calling |
@ChristopherHogan, this is officially the most insanely useful bug report in history. It would have taken me hours to track down the issue you identified. It is fixed in stevengj/libctl#14. I verified that your reproducer code fails in the current libctl master, but succeeds with the fix. I also updated the |
With stevengj/libctl#14, the prism and block runs are accurate to a tolerance of int main(int argc, char *argv[])
{
meep_geom::material_type mat = meep_geom::make_dielectric(12);
vector3 center = {0, -11.5, 0};
vector3 e1 = {1, 0, 0};
vector3 e2 = {0, 1, 0};
vector3 e3 = {0, 0, 1};
vector3 size = {1e20, 1, 1e20};
geometric_object block = make_block(mat, center, e1, e2, e3, size);
vector3 vertices[4] = {
{-8, -11, 0},
{8, -11, 0},
{8, -12, 0},
{-8, -12, 0}
};
vector3 axis = {0, 0, 1};
geometric_object prism = make_prism(mat, vertices, 4, 0, axis);
vector3 p = {6.9, -12, 0};
vector3 nhat_block = normal_to_object(p, block);
vector3 nhat_prism = normal_to_object(p, prism);
double tolerance = 1.0e-6;
if (!vector3_nearly_equal(nhat_block, nhat_prism, tolerance)) {
printf("FAIL: Normals not equal\n");
printf(" Block: {%f, %f, %f}\n", nhat_block.x, nhat_block.y, nhat_block.z);
printf(" Prism: {%f, %f, %f}\n", nhat_prism.x, nhat_prism.y, nhat_prism.z);
}
return 0;
} |
Changing the z size of the blocks from |
Ok, I reverted the scheme example back to using infinite sized blocks. The question is, how do we reproduce that geometry with prisms? I tried this: no_bend_vertices = [
mp.Vector3(-mp.inf, wvg_ycen - 0.5 * w),
mp.Vector3(+mp.inf, wvg_ycen - 0.5 * w),
mp.Vector3(+mp.inf, wvg_ycen + 0.5 * w),
mp.Vector3(-mp.inf, wvg_ycen + 0.5 * w)
]
geometry = [mp.Prism(no_bend_vertices, mp.inf, material=mp.Medium(epsilon=12))] but it produced a completely black epsilon file (all ones). |
I should also add that the reason I had replaced the infinite sizes with zeros was because that's what #341 did. I guess it needs to be fixed too. |
How do the errors in the bend case (compared to running at a very high resolution) compare for the block vs prism case now? Are they both on the same order? |
Yes, it looks better now. |
This reverts commit 0203ab4aa1fc164e9f3b747a84e0b97280bbbd70.
Ardavan requested that I update the |
* Add python Prism wrapper * Increase tolerance and reuse compare_arrays from tests/mpb.py * Simplify vector assignment * Get new scheme data with fixed size blocks * Revert "Get new scheme data with fixed size blocks" This reverts commit 0203ab4aa1fc164e9f3b747a84e0b97280bbbd70. * height is double * Extend prism out of cell and increase height.
compare_arrays
into a utils module so it's available for all tests.python/tests/bend_flux.py
with prisms. I had to increase the tolerance from1e-7
to1e-2
for thebend_flux.py
test to pass. Is that alarming or expected?@stevengj @oskooi @HomerReid