Skip to content

feature/sensitivity mask#1061

Merged
Jammy2211 merged 29 commits intomainfrom
feature/sensitivity_mask
Oct 16, 2024
Merged

feature/sensitivity mask#1061
Jammy2211 merged 29 commits intomainfrom
feature/sensitivity_mask

Conversation

@rhayes777
Copy link
Copy Markdown
Collaborator

Resolves #1058

@Jammy2211
Copy link
Copy Markdown
Collaborator

A consequence of the masked result is that code which uses the result no longer works, as expected.

For example, autolens uses the following code to get the y coordinate of every subhalo:

    @property
    def y(self) -> List[float]:
        """
        The y coordinates of the physical values of the subhalo grid, where each value is the centre of a grid cell.

        These are the `centre` coordinates of the dark matter subhalo priors.
        """
        return self.physical_centres_lists_from(
            path="galaxies.subhalo.mass.centre.centre_0"
        )

This now gives an error because certain Samples objects are None in the autofit code called:

Traceback (most recent call last):
  File "/mnt/c/Users/Jammy/Code/PyAuto/autolens_workspace_test/slam/integration/source_lp/mass_total/sensitivity.py", line 357, in <module>
    fit()
  File "/mnt/c/Users/Jammy/Code/PyAuto/autolens_workspace_test/slam/integration/source_lp/mass_total/sensitivity.py", line 342, in fit
    subhalo_result = slam.subhalo.sensitivity_imaging_lp.run(
  File "/mnt/c/Users/Jammy/Code/PyAuto/autolens_workspace_test/slam/subhalo/sensitivity_imaging_lp.py", line 798, in run
    subhalo_util.visualize_sensitivity(
  File "/mnt/c/Users/Jammy/Code/PyAuto/autolens_workspace_test/slam/subhalo/subhalo_util.py", line 136, in visualize_sensitivity
    mat_plot_2d = aplt.MatPlot2D(axis=aplt.Axis(extent=result.extent), output=output)
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoLens/autolens/lens/sensitivity.py", line 66, in extent
    return (np.min(self.x), np.max(self.x), np.min(self.y), np.max(self.y))
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoLens/autolens/lens/sensitivity.py", line 57, in x
    return self.perturbed_physical_centres_list_from(
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/non_linear/grid/sensitivity/result.py", line 43, in perturbed_physical_centres_list_from
    return self._physical_centres_lists_from(
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/non_linear/grid/grid_list.py", line 36, in wrapper
    values = func(grid_search_result, *args, **kwargs)
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/non_linear/grid/grid_search/result.py", line 69, in _physical_centres_lists_from
    return [value_for_samples(samples) for samples in samples]
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/non_linear/grid/grid_search/result.py", line 69, in <listcomp>
    return [value_for_samples(samples) for samples in samples]
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/non_linear/grid/grid_search/result.py", line 61, in value_for_samples
    return samples.model.object_for_path(path).mean
AttributeError: 'NoneType' object has no attribute 'model'

The code raising the error is this:

        if isinstance(path, str):
            path = path.split(".")

            def value_for_samples(samples):
                return samples.model.object_for_path(path).mean

        else:
            paths = [p.split(".") for p in path]

            def value_for_samples(samples):
                return tuple(samples.model.object_for_path(p).mean for p in paths)

        return [value_for_samples(samples) for samples in samples]

There is a similar issue where the result is None but when code which calls the log_evidence's of the sensitivity map is caleld, it causes a bug:

  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/non_linear/grid/sensitivity/result.py", line 82, in log_evidence_differences
    self.log_evidences_perturbed, self.log_evidences_base
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/non_linear/grid/grid_list.py", line 36, in wrapper
    values = func(grid_search_result, *args, **kwargs)
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/non_linear/grid/sensitivity/result.py", line 71, in log_evidences_perturbed
    return [sample.log_evidence for sample in self.perturb_samples]
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/non_linear/grid/sensitivity/result.py", line 71, in <listcomp>
    return [sample.log_evidence for sample in self.perturb_samples]
AttributeError: 'NoneType' object has no attribute 'log_evidence'

Could we make it so that every masked sensitivity grid cell is given a NullSamples object, which has the following:

  1. The model, so that the above code still works even though the fit is bypasssed.
  2. A log_evidence and log_likelihood of zero, which I think is a sensible enough default value.

@rhayes777
Copy link
Copy Markdown
Collaborator Author

Done

@Jammy2211
Copy link
Copy Markdown
Collaborator

The model object is there but could we have it also include the perturb?

Traceback (most recent call last):
  File "/mnt/c/Users/Jammy/Code/PyAuto/autolens_workspace_test/slam/integration/source_lp/mass_total/sensitivity.py", line 357, in <module>
    fit()
  File "/mnt/c/Users/Jammy/Code/PyAuto/autolens_workspace_test/slam/integration/source_lp/mass_total/sensitivity.py", line 342, in fit
    subhalo_result = slam.subhalo.sensitivity_imaging_lp.run(
  File "/mnt/c/Users/Jammy/Code/PyAuto/autolens_workspace_test/slam/subhalo/sensitivity_imaging_lp.py", line 798, in run
    subhalo_util.visualize_sensitivity(
  File "/mnt/c/Users/Jammy/Code/PyAuto/autolens_workspace_test/slam/subhalo/subhalo_util.py", line 136, in visualize_sensitivity
    mat_plot_2d = aplt.MatPlot2D(axis=aplt.Axis(extent=result.extent), output=output)
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoLens/autolens/lens/sensitivity.py", line 66, in extent
    return (np.min(self.x), np.max(self.x), np.min(self.y), np.max(self.y))
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoLens/autolens/lens/sensitivity.py", line 57, in x
    return self.perturbed_physical_centres_list_from(
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/non_linear/grid/sensitivity/result.py", line 43, in perturbed_physical_centres_list_from
    return self._physical_centres_lists_from(
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/non_linear/grid/grid_list.py", line 36, in wrapper
    values = func(grid_search_result, *args, **kwargs)
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/non_linear/grid/grid_search/result.py", line 69, in _physical_centres_lists_from
    return [value_for_samples(samples) for samples in samples]
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/non_linear/grid/grid_search/result.py", line 69, in <listcomp>
    return [value_for_samples(samples) for samples in samples]
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/non_linear/grid/grid_search/result.py", line 61, in value_for_samples
    return samples.model.object_for_path(path).mean
  File "/mnt/c/Users/Jammy/Code/PyAuto/PyAutoFit/autofit/mapper/model.py", line 186, in object_for_path
    instance = getattr(instance, name)
AttributeError: 'Collection' object has no attribute 'perturb'

@Jammy2211
Copy link
Copy Markdown
Collaborator

So I think the issue is a SensitivityResult has two methods:

    def perturbed_physical_centres_list_from(
        self, path: Union[str, Tuple[str, ...]]
    ) -> GridList:
        """
        Returns the physical centres of the perturbed model for each sensitivity fit

        Parameters
        ----------
        path
            The path to the physical centres in the samples
        """
        return self._physical_centres_lists_from(
            self.perturb_samples,
            path,
        )
    # noinspection PyTypeChecker
    @as_grid_list
    def physical_centres_lists_from(
        self,
        path: Union[str, Tuple[str, ...]],
    ) -> GridList:
        """
        Get the physical centres of the grid search from a path to an attribute of the instance in the samples.

        Parameters
        ----------
        path
            The path to the attribute to get from the instance

        Returns
        -------
        A list of lists of physical values
        """
        return self._physical_centres_lists_from(self.samples, path)

I think as long as the model includes the perturb they should all work.

@rhayes777
Copy link
Copy Markdown
Collaborator Author

Added. I guess the two options are we carry on mocking the same interface or we could consider if there's an elegant way to account for masking in downstream functions?

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.81%. Comparing base (410a25a) to head (6cea6f7).
Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1061      +/-   ##
==========================================
+ Coverage   80.60%   80.81%   +0.21%     
==========================================
  Files         199      199              
  Lines       15305    15358      +53     
==========================================
+ Hits        12336    12411      +75     
+ Misses       2969     2947      -22     
Flag Coverage Δ
?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Jammy2211
Copy link
Copy Markdown
Collaborator

Jammy2211 commented Oct 16, 2024

I did initially try to incorporate the mask downstream, but couldn't find an elegant solution and felt like it kept sending me back up to the original sensitivity mapping results.

The problem was that these calculations often use the shape and structure of the sensitivity grid, which was not available in the SensitivityResult after masking, but I think now is.

@Jammy2211
Copy link
Copy Markdown
Collaborator

The values coming from perturbed_physical_centres_list_from are still zero'd at masked values.

However, they come from the model and as far as I can tell should still be computable via this function:

        if isinstance(path, str):
            path = path.split(".")

            def value_for_samples(samples):
                return samples.model.object_for_path(path).mean

        else:
            paths = [p.split(".") for p in path]

            def value_for_samples(samples):
                return tuple(samples.model.object_for_path(p).mean for p in paths)

Just because we apply a mask and skip the fit, the perturb model can still be set up with a prior for these values allowing the function above to return a non-zero value, I think?

Basically, to produce 2D images of the sensitivity result for autolens, I need to know the x and y coordinates of where the sensitivity mapping of a dark matter subhalo took place (including cells which were bypassed via a mask). I don't think there is a general way I can do this (e.g. independent of how the sensitivity grid was set up) without calling:

   @property
   def y(self) -> af.GridList:
       """
       The y coordinates of the physical values of the sensitivity mapping grid.

       These are the `centre` coordinates of the dark matter subhalos that are included in the simulated datasets.
       """
       return self.perturbed_physical_centres_list_from(
           path="perturb.mass.centre.centre_0"
       )

   @property
   def x(self) -> af.GridList:
       """
       The x coordinates of the physical values of the sensitivity mapping grid.

       These are the `centre` coordinates of the dark matter subhalos that are included in the simulated datasets.
       """
       return self.perturbed_physical_centres_list_from(
           path="perturb.mass.centre.centre_1"
       )

@Jammy2211
Copy link
Copy Markdown
Collaborator

I could, for example, extract this information from some of the internal sensitivty mapping arrays, but if the parameter ordering were swapped by the user this calculation would no longer work correctly, so it needs to work on the model path.

@rhayes777
Copy link
Copy Markdown
Collaborator Author

Could you not simply check whether a result is a MaskedJobResult and skip setting the associated values in that case?

@rhayes777
Copy link
Copy Markdown
Collaborator Author

It looks as though these perturbed values are being respected for masked results

def test_physical_centres_lists(masked_result, masked_sensitivity):
    assert masked_result.perturbed_physical_centres_list_from("perturb.centre") == [
        0.25,
        0.25,
        0.25,
        0.25,
        0.75,
        0.75,
        0.75,
        0.75,
    ]

When prior limits were updated for perturbed models this was not updating the underlying message for Uniform and LogGaussian priors causing the mean to be the same for every perturb model inspite of the priors differing. Was this the cause of the issue?

@rhayes777
Copy link
Copy Markdown
Collaborator Author

The perturbed_physical_centres_list_from now takes values directly from the sensitivity map result. No need to specify 'perturb'.

e.g.

masked_result.perturbed_physical_centres_list_from("centre") 

@Jammy2211 Jammy2211 merged commit f7d510b into main Oct 16, 2024
@Jammy2211 Jammy2211 deleted the feature/sensitivity_mask branch February 10, 2025 19:11
@Jammy2211 Jammy2211 restored the feature/sensitivity_mask branch February 10, 2025 19:11
@Jammy2211 Jammy2211 deleted the feature/sensitivity_mask branch March 24, 2025 19:41
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.

Masked Sensitivity Mapping

2 participants