Point Source handling

[gvernard]

Updating the way a point source is stored in COOLEST.

[gvernard]

I added the variables to the class but I am facing two problems now: 1) I don't know how to set the entries in init for the flag parameters, and 2) I would assume that each class has a constructor that takes the parameters as input and checks whether they make sense or not, e.g. a negative flux wouldn't. This is were we would ideally check the given true or lensed or both sets of properties and make sure they match the flags (or set the flags if not provided).

[gvernard]

Note for future development: once we implement a 'lenser' code, for example, Herculens working in the backend (useful to create caustics as well), and if the 'flag_coupled' is given by the user, then we should check whether the true and lensed point source positions are compatible. We could do the same for the magnification.

[gvernard]

Fixed problem 1 from above. The final remaining issue is what units should we give to the point source brightness, flux or mag?

[aymgal]

Why are the flags actual parameters of the light profile?
They could just be additional attributes of the class.

[gvernard]

I don't know exactly how to implement this. I looked but didn't find any example to follow in profiles/light.py or profiles/mass.py. Can you fix this?

[aymgal]

@gvernard thanks for the PR! I reviewed the changes and added some comments (see above).

So concerning the flags, I would simply not have them as profile parameters, but simply as additional python attributes (these would automatically appear in the template file then). However I am not sure about the need for two flags. Wouldn't it be easier to have a single flag (lensed or unlensed), and assume that if it is lensed, then the "intrinsic" and "lensed" parameters are coupled? What would be a use case where they are not coupled?

About the lenser, in the API there is aready ray-tracing implemented (and thus the caustics). It is accessible using the ComposableLensModel class.

[gvernard]

I implemented all of the comments except the one for using class attributes instead of light profile parameters for the flags.

The point about the lenser makes sense, and it should be straightforward to check the magnification of a point source (a bit more tricky for the intrinsic and lensed positions). But my question is how can we implement some assertions when the object is initialized. For example, a function (the constructor? init()?) that would check that the given intrinsic and lensed parameters are consistent with a given mass model, and return an error if they are not.

[aymgal]

@gvernard thanks for the changes, I will push soon some additional changes to the PR and we'll discuss further from there.

[aymgal]

@gvernard Ok I made significant improvements in the way the COOLEST instance is being checked when loaded with the load() method from coolest.template.json.

Essentially you can now pass two new settings check_point_sources (True/False) and source_plane_tolerance (by default 1e-3 arcsec in source plane), and this will trigger the self-consistency check between intrinsic and lensed positions. If you use the get_coolest_object in coolest.api.util to load the template, then these new arguments should be passed as a dictionary instead (e.g., kwargs_validator={check_point_sources=True}).

I also moved your flag_contains to a property of the PointSource class, such that it can be used to dynamically check what the point source profile contains. One catch is that flag_contains will then not appear in the JSON template file, but it's fine since the user can anyway see of the profile contains both or only intrinsic/lensed positions.

IMPORTANT: I did not check that it actually works on a template that contains point sources. Since you have this already, I leave the rest of the implementation and final checks to you, but I think the current code should be a good basis to add and fix things if needed.

Let me know if anything is unclear!

[gvernard]

Great, thanks for implementing these requests! I reviewed the changes you made regarding declaring and initializing a point source and it looks fine to me. I fixed a typo and I noticed that the notebook example in the documentation page (https://coolest.readthedocs.io/en/latest/notebooks/02-generate_template.html) is not up-to-date with the repo (ah, but it could be because this is a different branch...). I will test this functionality with an actual point source (hopefully soon) and once it hopefully succeeds, then we can close this pull request.

[aymgal]

@gvernard yes the notebook has been updated only on the PR branch so that's ok.
Alright, let me know!

[aymgal]

Hi @gvernard, what is the status with this?