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

Add sidewall angle parameter #53

Merged
merged 64 commits into from
Feb 11, 2020
Merged

Conversation

danielwboyce
Copy link
Contributor

Fixes NanoComp/meep#1067.

  • Adds sidewall angle parameter to prism object, which allows user to define a draft angle at which their prism object is tapered during extrusion.

  • Adds additional properties to prism object to store vertices of the top polygon.

  • Helper functions in geom.c like geom_object_volume have been updated to work with new structure of the prism object.

…sm_vector_* calculations because the vectors that are being transformed don't have a particular location. We'll need to try something else.
…e of a prism object; the list of derived values may be shorted later.

* Addition of a comment to geom.scm giving a brief explanation of what sidewall_angle does.
…om_p.

* Added derived quantities vertices_top and vertices_top_p to prism subclass (calculations to derive these quantities have not yet been added).
* Misc. changes to update functions to work better with top polygon/bottom polygon framework.
* Update to normal to prism for compatibility with top polygon framework.
…work. To do so, I took the RMS area of the bottom polygon area and top polygon area then multiplied by the height of the prism.
…ound and then vertices are calculated from the intersection of those edges.
@danielwboyce
Copy link
Contributor Author

danielwboyce commented Feb 10, 2020

With the most recent commit we have all the critical features for this merge in place:

  • Sidewall angle in prism constructor
  • Helper functions are updated for prisms with a sidewall angle
  • Much more comprehensive test suite has been added.

Running make check a few thousand times has it failing only about 5.9% of the time. I'm working on diagnosing those issues in case we want better performance.

@danielwboyce
Copy link
Contributor Author

Running make check a few thousand times has it failing only about 5.9% of the time. I'm working on diagnosing those issues in case we want better performance.

This actually seems to be a bug I had run into before. Each time make check runs it does 1,000 tests to find the intersection length between a line segment and a prism, checking against the intersection length found when the prism is constructed as a block. (This is unit test 3 in utils/test-prism.c.) When I investigated this before, I found that whenever make check fails it's due to one of the randomized intersection tests having a small floating point error (of about 1%, but the typical threshold is 1 part in a million). My judgement is that this is a small enough error that we're probably still fine to merge.

utils/geom.c Outdated Show resolved Hide resolved
utils/geom.c Show resolved Hide resolved
@stevengj stevengj merged commit 79c0af7 into NanoComp:master Feb 11, 2020
@stevengj
Copy link
Collaborator

Whoops, looks like this is breaking the Meep build because the typemap utils explicitly reference the prism->vertices list here.

We should undo the rename of vertices to vertices_bottom since that is breaking.

@stevengj
Copy link
Collaborator

vertices_bottom renamed in 7f31263

@stevengj
Copy link
Collaborator

@danielwboyce, Meep now compiles again, but the mode_coeffs test is failing, and I'm guessing this may relate to the prism handling. Can you take a look?

@thchr
Copy link
Contributor

thchr commented Feb 13, 2020

I think this is breaking the MPB CI builds because variable declaration inside for loops is not C89 compatible. See https://travis-ci.org/NanoComp/mpb/jobs/649653306#L1075-L1087 (build failure in NanoComp/mpb#112).

I can make a PR to fix this if there's agreement with the above.

@stevengj
Copy link
Collaborator

stevengj commented Feb 13, 2020

@thchr, a PR to fix the declarations to be C90-compliant would be welcome.

(Would also be good to exercise this in the tests.)

bmwiedemann added a commit to bmwiedemann/openSUSE that referenced this pull request Aug 17, 2020
https://build.opensuse.org/request/show/827344
by user badshah400 + dimstar_suse
- Update Source URL, moved to https://github.com/NanoComp/libctl.
- update to 4.5.0:
  * New `make_slanted_prism` functions to make a prism with
    a given sidewall angle (gh#NanoComp/libctl#53).
  * Defined `LIBCTL_MAJOR_VERSION` etc. in `ctlgeom.h` header file
    when using stand-alone libctlgeom.
  * Bugfix in point-in-prism test (gh#NanoComp/libctl#49).
  * `geom_object_volume` function to get the volume of a 3d
    object (accelerates `box_overlap_with_object` for objects
    completely within a box) (gh#NanoComp/libctl#45).
  * `ctl_printf_callback` so that callers can capture stdout
    (gh#NanoComp/libctl#39).
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.

Adding sidewall angle parameter to polygon object
4 participants