Separate out the geometry from the functional optics #182
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In this commit: - Get geometry information from ['XXX'] syntax - Call geometry functions - Adapt CircularDetector (the only object that used a cylindrical geometry before) - Modify tests to use forward rays (some tests required rays to go backwards to hit elements) All but one tests pass at this point, but more work is needed ot make things consistent and update documentation and comments.
In some cases I just silenced certain warnings (e.g. the ACIS dimensions are not an integer multiple of the pixel size. That is well known by the developers but there is no reason to tell a user any time since they can't really change it), in other cases code was rewritten to handle edge cases better (e.g. testing for equality of vectors before deviding them by their cross-product to avoid "devide by 0" messages).
Replace display dicts with a version that looks up missing items in geometry. In this way, useful defaults can be taken fomr the geometry (e.g. the shape) withou actually making a copy of the data (which risks not being up to date in something is changed in the geometry). This works for now and is a step forward, but might still evolve further before the next release, e.g. - use traitlets to allow default for color - copy reference to value into display dict, taking away the need to define a special class. These options need to be investigated.
This allows for cylindrical detectors again.
- Global EnergyFilter does not need a Geometry - Defaults for plotting surface mesh for Cylinder were wrong
FlatGrating should not work with different geometries, not just a flat plane.
|
This code is not complete in the sense that not all objects are fully supported in the new geometry scheme. However, I'll get ready to merge it at this point. Separate issues and PRs can be used to finish this. At this point, I;d rather get this branch in line with master again so that I can adapt my Arcus code for it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This partially addresses #181