BXA
BXA copied to clipboard
ingest David Homan's Bayesian workflow in docs
PR https://github.com/JohannesBuchner/BXA/pull/51 merged code that shows nice notebooks.
We should
- add these on the website under "Bayesian workflow" using nb-sphinx, so that they are run when the documentation is built
- link from the BXA/xspec manual, so that users can switch to it
- add them to the CI, so that the workflow is run and is ensured to work
@dshoman could you have a look at the following issues, please?
- Readme should include contact
- python scripts need
pip install natsort
which is not yet described in the readme - the tutorial_usage_plotxspec.ipynb notebook references files not generated by run_all.sh:
"epicfn = 'epic_pn_agn_grp1.fak'\n",
"mos1fn = 'epic_mos1_agn_grp1.fak'\n",
"mos2fn = 'epic_mos2_agn_grp1.fak'\n",
Maybe the grouping command is missing? Should we go for grouping by 3 (or 5) counts to avoid the biases pointed out in https://giacomov.github.io/Bias-in-profile-poisson-likelihood/ (also referenced in the book chapter)
Otherwise it all ran through on my laptop.
@JohannesBuchner of course, I will add the updates in one new pull request.
-
For the
natsort
requirement, should this be mentioned in the main readme, or in the readme in the bayesian-workflow directory? -
The creation of the files "epic_[...]_grp.fak" is part of the commands in
gen_xmm.xspec
and relies ongrppha
. So the commands are there, but they are not running correctly on your system. Perhaps is it wrong to assume thatgrppha
is available to everyone runningxspec
. If so, the easiest solution is probably to simply drop the grouping altogether? I have tested the results on my system and everything also works well without the grouping.
@JohannesBuchner of course, I will add the updates in one new pull request.
Yes, please.
* For the `natsort` requirement, should this be mentioned in the main readme, or in the readme in the bayesian-workflow directory?
The latter.
* The creation of the files "epic_[...]_grp.fak" is part of the commands in `gen_xmm.xspec` and relies on `grppha`. So the commands are there, but they are not running correctly on your system. Perhaps is it wrong to assume that `grppha` is available to everyone running `xspec`. If so, the easiest solution is probably to simply drop the grouping altogether? I have tested the results on my system and everything also works well without the grouping.
Ah, sorry. I have ftgrouppha from heasoft, not grppha, but I will see if I should have added something to my configure command during heasoft install. If there is no background, do we need grouping? If there is a background, shouldn't we group by the background spectrum?
Ah, sorry. I have ftgrouppha from heasoft, not grppha, but I will see if I should have added something to my configure command during heasoft install.
For my install, both ftgrouppha and grppha are present. As this is not a given, I would say the safer option would be not to assume they are installed for all users.
If there is no background, do we need grouping? If there is a background, shouldn't we group by the background spectrum?
No, the grouping is not necessary. I left out a background spectrum for simplicity. This could be a nice addition though, to show how this is included in the plotting options. For now, I suggest to leave the grouping (and background) out.
Please have a look at https://johannesbuchner.github.io/BXA/ for how it looks now and let me know if you want any changes.
Thanks very much for including the notebooks in the documentation, it looks good to me.
There is some unusual output of the PlotBXA notebook, when creating the fakeit spectra (cell 37). There is an XSpec error regarding the file names of the spectra. I am not able to reproduce this on my local systems. It could be OK to simply suppress the output from this cell, as it has no direct input for the rest of the notebook (and as far as I can tell, the simulated files are still generated).
One thing I don't particularly like in the current navigation on the left is that the titles are so long and there is no clear separation. Bullet points would help. Not sure how to fix that at the moment.
I think the title length should be an easy fix: at the moment only the main header in the notebook (i.e. # ...) is taken into the selection menu. We could simply move the subtitles to a lower level of header (## ... ), that should reduce the entries in the navigation menu to simply 'PlotBXA example' and 'PlotXspec example', respectively.