Add reasonable default magnification methods for all microlensing configurations in `Model`

[ketozhang]

Currently, it is up to the user to set the magnification method on Model (e.g., this tutorial), with the method Model.set_default_magnification_method(). This is only required sometimes (e.g., not point-source single-lens).

# This makes sense
model = mm.Model({...})
model.get_lc(...) # OK

# Add rho
mm.Model({..., "rho": 0.0001})
model.get_lc(...) # BAD, you forgot to set magnification method

# Method 1
model=mm.Model({..., "rho": 0.001})
model.set_default_magnification_method("finite_source_uniform_Gould94")
model.get_lc(...) # OK

# Method 2
model=mm.Model({..., "rho": 0.001})
model.set_magnification_method([0, "finite_source_uniform_Gould94", 100])
model.get_lc(...) # OK

In my opinion the process of doing this is not nice, and it isn't immediately clear to the user: (1) you must use either setter methods (warning isn't clear about your option) ; (2) you must find the list of methods available (this is pretty hard to find without familiarity). I'd much prefer these information is available upfront.

The suggestion I am making is to make these all available to the constructor

class Model():

    def __init__(self, parameters, ..., magnification_method: str | list | None  = None):
        # Need docstring to list all the method for magnification_method. This is where the documentation 
        # should exist, not under `get_point_lens_magnification`.
        ...
        if magnification_method is None:
            self.set_default_magnificaiton_method(
                infer_default_magnification_method_from_parameters(parameters)
            )

# Usage
model = mm.Model(..., magnification_method="finite_source_uniform_Gould94")

# Construct a model with subset of time ranges for finite-source otherwise point-source.
model = mm.Model(..., magnification_method=[30, "finite_source_uniform_Gould94", 50])  

[rpoleski]

We can definitely improve the warnings. The problem I see with what you proposed is that __init__() options don't provide the same functionality: either default method or a list of methods can be provided. This needs some more thinking.

[ketozhang]

I've ended up needing to code this myself and it does support passing in single method str, or a list of methods

    def __init__(self, ..., magnification_method: str | Iterable | None = None)
        if magnification_method is None:
            magnification_method = self._infer_default_magnification_method()

        if isinstance(magnification_method, str):
            self.set_default_magnification_method(magnification_method)
        elif iter(magnification_method):
            self.set_magnification_methods(list(magnification_method))
        else:
            raise ValueError(
                "Arg `magnification_method` must be str, Iterable, or None."
            )

It works with one missing feature, the ability to set both the default and time range method. I'd argue doing so is an advanced usage of this package and should be provided outside the constructor.

---

Implementation can be changed, let's discuss whether or not we'd like a better warning message and/or a different interface.

[jenniferyee]

I agree the current implementation is complicated. However, I prefer something that parallels the current API:

mm.Model(default_magnification_method='point_source', methods=[8300., 'finite_source_uniform_Gould94', 8310.])

I also agree that it is annoying to have to figure out the available magnification methods. Perhaps we could add a method:

mm.model.print_available_magification_methods()

or

mm.Model().print_available_magnification_methods()

is the second one allowed without defining the model?

[ketozhang]

mm.Model(default_magnification_method='point_source', methods=[8300., 'finite_source_uniform_Gould94', 8310.]) That looks good to me with one change methods -> magnification_methods

mm.Model().print_available_magnification_methods()

is the second one allowed without defining the model?

Yes, as a class method, decorator @classmethod.

However, do you want to provide a superset of methods (single + binary lens)? If not, then you will need to infer the number of lenses from the parameters which are only available after construciton.

I advise against print functions. They're not pythonic outside of repr and str. Static class/instance attributes are better (if dynamic, use property). The most common way to do this is to have a composite data object available to be imported.

define an Enum

from enum import Enum
from typing import Union

class SingleMagnificationMethod(Enum):
    POINT_SOURCE = "point_source"
    FINITE_SOURCE_GOULD = "finite_source_uniform_Gould94" 
    FINITE_SOURCE_GOULD_DIRECT = "finite_source_uniform_Gould94" 
    ...

class BinaryMagnificationMethod(Enum):
    ...

def set_default_magnification_method(
    method: Union[SingleMagnificationMethod, BinaryMagnificationMethod]
):
    if (
        not isinstance(method, SingleMagnificationMethod) 
        or not isinstance(method, BinaryMagnificationMethods)
    ):
        raise ValueError(...) 

, or use type hinting

# Requires Python 3.8+
from typing import Literal

# Define a type to be a limited set of strings
SINGLE_MAGNIFICAITON_METHOD: Literal[
    "point_source",
    "finite_source_uniform_Gould94",
    ...
]

...
def set_default_magnification_method(
    method: Union[SINGLE_MAGNIFICAITON_METHOD, BINARY_MAGNIFICAITON_METHOD]
):
    # NOTE: `method` is still str, the type hinting only informs the users they can choose a limited set.
    ...

With either these two method, it is very simple to hyperlink them with Sphinx.

[ketozhang]

This is my fault, I've open the scope to design interface (which can be discussed on a separate ticket). Narrowing down to the original issue, can we add reasonable defaults:

• PSPL
  • No changes needed
• FSPL
  • Set default method to one of the finite source single lens methods
  • If gamma parameter set, set default to a LD finite source method[^1]
• Binary lens
  • Set default method to one of the binary lens methods, VBBL?

[^1]: It isn't perfect, I'd prefer the magnification method to be known after construction of Model, and not to change because I decided to add gamma. This is indicative that gamma should initialized during construction.

[rpoleski]

Long time ago we have discussed default methods for finite sources. The agreement was to not provide them, because any method can be either scienfitifically wrong or inefficient in quite large number of cases. Also we want to encourage users to use different methods.

[rpoleski]

Warning text corrected in e3a45e7.

[rpoleski]

f466009 adds further explanation.