Sitelet https://github.com/bandframework/bandframework/pull/175
Skip to content

Adding submodule ModelDiscrepancy to examples folder - #175

Merged
wildsm merged 9 commits into
bandframework:developfrom
sjaiswal-tifr:develop
Nov 7, 2025
Merged

wildsm merged 9 commits into
bandframework:developfrom
sjaiswal-tifr:develop

Conversation

@sjaiswal-tifr

Copy link
Copy Markdown
Contributor

PR to add the submodule to examples/ModelDiscrepancy.

@wildsm wildsm mentioned this pull request Oct 27, 2025
22 tasks done
Added ModelDiscrepancy example to README.
Added a new example entry for ModelDiscrepancy with a link to its repository.
CI, after it was updated to reflect new BAND structure) caught a missing space.
@wildsm

wildsm commented Oct 27, 2025

Copy link
Copy Markdown
Member

I've made edits to the bandframework files that would reference ModelDiscrepancy.

I also corrected a typo currently in develop (so if this PR goes away, we need to revisit that).

I ensured that actions are passing with these changes.

Should be ready for reviewers

@asemposki

Copy link
Copy Markdown
Member

I am starting my review.

I followed the README instructions to make the Conda environment needed and to test the package using the two test files. Both of these passed without any issues.

I then went into the projects folder and opened the Jupyter notebook run.ipynb in the arXiv_2504.13144 folder. I had some issues getting Jupyter notebook to correctly connect to a kernel, but this was a mismatched Conda/homebrew issue, which is not a problem with this package but a problem with my setup.

Once I fixed that issue, I was able to run the ball drop notebook without any problems. I also then opened the plots.ipynb notebook and everything here also loaded up and executed correctly. I did not run every notebook, as the amount of energy this took drained my laptop pretty quickly. However, things seem to be working so if @DanielRPhillips would like to run a different set of notebooks, that would cover more of the Jupyter notebooks in the package.

From these checks, things look good to me! Very interesting work, as well!

@DanielRPhillips

Copy link
Copy Markdown
Member

I conducted my review of the ModelDiscrepancy package. I cloned the repo onto my laptop. I followed the instructions in the README to install the environment file, and then launched anaconda under that environment. I ran the tests listed in the README and they both passed.

I then investigated the structure of the repo, and ran the various notebooks that reproduce results from publications. The repo is well structured. It is nice that, for each of the two research papers, there is the opportunity to regenerate the plots in the paper and the data from sampling is provided. The notebooks that consider heavy-ion examples,
arXiv_2504.13444/heavy-ion/5param_model/plots_shear0p1_esw0p2.ipynb
arXiv_2504.13444/heavy-ion/5param_model/plots_etakink0p1_Tkink0p18_ahigh1_alow-1_esw0p2.ipynb
arXiv_2504.13444/heavy-ion/2param_model/plots.ipynb ("Heavy-ion: 2-parameter model. Plots & reproduction guide") and
arXiv_2509.19759/plots.ipynb (The plots for the second of the two projects provided)
have clear descriptions of what each notebook does to generate the pertinent plots. They also contain instructions on how to re-generate the results of the sampling, if so desired.

All these ran to completion, with only some warnings related to the figure layout.

Similarly, the notebook that generates plots for the ball-drop example arXiv_2504.13144/ball-drop/plots.ipynb, has a good explanation of what to do in the first cell. It too ran to completion

In the ball-drop case the inference can be straightforwardly run by executing run.ipynb in the same directory. This file could perhaps use a little more explanation of what it is doing in the top-level cell. Maybe a pointer to the paper whose results it supports, and a statement that plots.ipynb should be used for plotting after run.ipynb has been executed?

I initially thought that more explanation of the workflow for the different examples presented in the repo could be useful. That explanation is not present in the top level README.md. But I now see that the workflow is all there in README.md files provided under each of the two projects (arXiv_2504.13444 and arXiv_2509.19759). Maybe a pointer to those README's at the top level would help?

All the notebooks are well structured with clear sections whose title describe what the code in that section is designed to do. Although the ability to intersperse text through the Jupyter notebook is not fully exploited, the cells include plenty of comments which make it clear what each piece of code is supposed to do. For example, this allowed me to uncomment a piece of code and successfully generate a corner plot of HI parameters.

I did find one small typo, in arXiv_2504.13144/ball-drop/run.ipynb, in the text headed "Sampling" between Cells 8 & 9, the word "quantiles" got mistyped as "wuantiles".

Overall this is a thoughtfully put together and clearly presented package. And, as already commented by @asemposki the results are very instructive! The improvement in g extraction when model discrepancy is considered is revelatory!

@DanielRPhillips DanielRPhillips left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These changes to include ModelDiscrepancy in v0.5 are fine

@wildsm
wildsm merged commit 07bc222 into bandframework:develop Nov 7, 2025
2 checks passed
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.

4 participants