Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/result cast #989

Merged
merged 81 commits into from
Apr 8, 2024
Merged

Feature/result cast #989

merged 81 commits into from
Apr 8, 2024

Conversation

Jammy2211
Copy link
Collaborator

New API for how the Analysis class returns a Return.

Now, the Analysis class has a Result class attribute, which a user can overwrite with their own custom Result class.

This removes a number of unecessary make_result functions that end up populating Analysis objects and is a cleaner API for users, as shown in the docs updated in this PR.

Copy link
Owner

@rhayes777 rhayes777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to fix those interpolator tests


residual_map = self.data - model_data_1d
chi_squared_map = (residual_map / self.noise_map) ** 2.0
log_likelihood = -0.5 * sum(chi_squared_map)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just return this line rather than defining an ephemeral variable

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for the example module, which users might read, so intentionally simple.


self._instance = instance or ModelInstance()
self._samples = samples or MockSamples(
max_log_likelihood_instance=self.instance, model=model or ModelMapper()
# max_log_likelihood_instance=self.instance,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented code

Comment on lines +42 to +46
def model_absolute(self, a):
try:
return self.samples_summary.model_absolute(a)
except AttributeError:
return self.model
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth adding a warning?

Comment on lines 305 to 311
try:
Samples.from_csv(
paths=self.paths,
model=self.model,
)
except FileNotFoundError:
pass
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to return this?

# )


# def test_interpolate(interpolator):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooof why is this all commented out?

@Jammy2211 Jammy2211 merged commit f71f9cd into main Apr 8, 2024
0 of 4 checks passed
@Jammy2211 Jammy2211 deleted the feature/result_cast branch April 18, 2024 12:44
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.

None yet

2 participants