xpsi-group / xpsi

X-PSI: X-ray Pulse Simulation and Inference
Other
34 stars 20 forks source link

Postprocessing bugfixes and updates #312

Closed thjsal closed 1 year ago

thjsal commented 1 year ago

I'll still check if there are other things that could be fixed with this request, but here is at least a fix to the problem when setting user-specified parameter limits for the corner-plots in the latest X-PSI versions using standard GetDist. After this fix, it is possible to set e.g. param_plot_lims={'radius':(10,16), 'mass':(1.0,2.5),}, without getting error messages.

thjsal commented 1 year ago

I added the option to set the plotted credible interval precision. Perhaps here are already enough changes to be reviewed and approved if they look ok.

yveskini commented 1 year ago

@thjsal, Would you like to include this additional line as a way to ensure that the user enters the correct length for the precision? This will help ensure that the number of parameters to plot matches the length of the precision list. If not, it would be better for xpsi to use the default settings instead of crashing.

if precisions is None or len(precisions) != plotter.subplots.shape[0]:
            precisions = [None]*plotter.subplots.shape[0]
thjsal commented 1 year ago

Your suggestion does not exactly work, since Python cannot check the length of precisions in case it is None (and it tries to check it even though the first condition would already be true). But I modified the code so that it indeed uses the default precision if wrong dimensions are provided (however, printing a warning in that case). And in case of providing something else than a list of integers, an exception is raised.

yveskini commented 1 year ago

@thjsal , The: raise ValueError("Precisions need to be given as a list of integers.") will make it crash if the if statement is not fulfilled which we don't want. We want to fall back to the default settings. So maybe:

print("Precisions need to be given as a list of integers."  +
    "Using the automatic default precision instead.")
thjsal commented 1 year ago

Oh, I see! I was not sure if that is always wanted since that could give the user the wrong impression that X-PSI is actually using the provided numbers, if not checking the printout. But I guess printing that information as a "warning" should be enough.