Structure of PointLens (and related) Classes #108
jenniferyee
started this conversation in
Ideas
Replies: 4 comments
-
see commit 7e1235b. |
Beta Was this translation helpful? Give feedback.
0 replies
-
Something to consider: How do these modifications interact with the needed modifications for gradient calculations? |
Beta Was this translation helpful? Give feedback.
0 replies
-
See #107 |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
For version3, I strongly advocate for subdividing the magnification calculations into separate/nested classes. Probably the place to do this is in the "PointLens"/"BinaryLens" classes. In reviewing the PointLens class and how it's called, there are a bunch of "problems" I would like to fix:
The naming of functions doesn't make sense. Specifying the magnification method "finite_source_uniform_Gould94" calls a function called "get_point_lens_finite_source_magnification," which makes it hard to match the magnification methods to the actual functions where the magnificaitons are calculated.
Not all of the individual functions in PointLens are directly tested (e.g. get_point_lens_uniform_integrated_magnification). This will make it harder to track down bugs.
Some of the finite source magnification calculations take pspl_magnification as an argument. This means the pspl_magnification has to be calculated separately. Also, these functions end up requiring multiple arguments, compared to the desired number = 1.
Why is there even a separate PointLens class at all? Why doesn't the PointLens class store the trajectory?
Beta Was this translation helpful? Give feedback.
All reactions