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

Added posterior predictive error on pulse model plotting #515

Merged
merged 6 commits into from
Feb 13, 2025

Conversation

lmauviard
Copy link
Collaborator

I added posterior predictive error bars to the modeled pulse profile plot (_residuals.py).

As before, I compute nsamples models and display the mean as the model. But now, I do not put the error bars on the data but I rather compute n_realisation_per_model (default=1) Poisson realizations of every model and use this to compute the error bars on the modeled pulse.
In the case with little realizations (n_realisation_per_model x nsmaples < 100), I plot the Poisson error on the modeled pulse instead. This is especially useful when the user want to do the plot with a given parameter vector.

Chi square is now no longer plotted on the pulse profile as it is not very relevant. But it can be accessed by plot.bolometric_chi2 if the residual plotter was run once.

The PostProcessing notebook was updated accordingly.

@lmauviard lmauviard added enhancement New feature or request postprocessing Issues or updates for postprocessing labels Jan 21, 2025
@lmauviard lmauviard self-assigned this Jan 21, 2025
@thjsal
Copy link
Contributor

thjsal commented Jan 29, 2025

I get
AttributeError: 'dict' object has no attribute 'bolometric_chi2
if trying to access the bolometric chi square in the way you said. Maybe the way to access it could be shown in the tutorial?

Quickly looking looks otherwise fine to me. Except that the legend text for "Data" and "Model" is on top of the curves. Can it be easily moved (e.g. for corner plot we can usually give legend_corner_coords)?

@thjsal
Copy link
Contributor

thjsal commented Jan 29, 2025

I also found it a bit confusing that you call different samples as different models. Would n_realisation_per_sample be a better name than n_realisation_per_model?

@lmauviard
Copy link
Collaborator Author

I did the changes you mentioned with bolometric chi2, n_realisation_per_sample and label positionning. I added a way to plot labels for non diagonal regions of posteriors if needed.

The PostProcessing notebook has been updated

@lmauviard
Copy link
Collaborator Author

Solves #522

@thjsal
Copy link
Contributor

thjsal commented Feb 10, 2025

I made some small additional changes to the post-processing notebook (fixing the issue with shaded credible regions), and ran it with an updated version so that it is clear which X-PSI version is needed to get the correct output (for the posterior predictive plots). I wrote something to the Changelog as well. You can sure change it if you want.

I also tried to additionally solve the issue #495 , but I was not sure if the solution was good, and did not implement it now.

Anyway, for me this looks fine to be merged to main (I suggest to "Squash and merge").

@lmauviard lmauviard merged commit 17d843b into xpsi-group:main Feb 13, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request postprocessing Issues or updates for postprocessing
Projects
None yet
2 participants