Skip to content

Sheshuk/fix model notebooks#402

Merged
JostMigenda merged 14 commits intomainfrom
sheshuk/fix_model_notebooks
Oct 7, 2025
Merged

Sheshuk/fix model notebooks#402
JostMigenda merged 14 commits intomainfrom
sheshuk/fix_model_notebooks

Conversation

@Sheshuk
Copy link
Copy Markdown
Contributor

@Sheshuk Sheshuk commented Jul 4, 2025

Closes #392 , #223

Trying to fix the remaining issues with building the notebooks:

Also, currently building the notebooks in the docs is not very informative: if one fails, we don't get any additional information. I'm adding a separate step, calling the pytest on notebooks, so we can see exctly which ones of them are failed, instead of exit on first failure.

@JostMigenda
Copy link
Copy Markdown
Member

[ ] Errors in AnalyticFluence - it's incompatible with our ccsn_loaders.

In #404, @jpkneller suggested (among other things) moving that class from ccsn.py to base.py. So we could leave this particular issue for that PR and don’t have to deal with it here.

Comment thread doc/source/conf.py Outdated
@JostMigenda
Copy link
Copy Markdown
Member

From discussion in the telecon just now:

  • Fornax_2019 requires more work to ensure units are correct, but that can happen outside of this PR here.
  • The other open issues will be tackled by @jpkneller in separate PRs

So I think we can turn this one from draft into a reviewable PR?
If you agree, I’ll try to review it later this week and merge it. (I’ll try to look at the following PRs by Jim as well, but can’t promise I’ll get to that before next week’s conference.)

@Sheshuk Sheshuk marked this pull request as ready for review September 4, 2025 18:38
Copy link
Copy Markdown
Member

@JostMigenda JostMigenda left a comment

Choose a reason for hiding this comment

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

Sorry for the delay! I didn’t get to this before the conference, then spent most of last week ill with some conference virus … 🤒

Aside from the separate discussion about nbsphinx_allow_errors above, I think there’s one more open issue here: In addition to the more exotic model notebooks, which we’ll leave for separate PRs, we also have FlavorTransformation.ipynb failing in the integration tests, with the following error:

TypeError                                 Traceback (most recent call last)
Cell In[6], line 1
----> 1 fig = plot_total_flux(model, AdiabaticMSW(MixingParameters()), AdiabaticMSW(MixingParameters('INVERTED')))
      2 fig.show()
      3 # fig.savefig('flux_adiabaticmsw.pdf')

Cell In[4], line 37
     35 for flavor in Flavor:
     36     for j, E in enumerate(energies):
---> 37         ispec[flavor][j] /= (4.*np.pi*d**2)
     38         ospec_nmo[flavor][j] /= (4.*np.pi*d**2)
     39         ospec_imo[flavor][j] /= (4.*np.pi*d**2)

TypeError: unsupported operand type(s) for /: 'd2NdEdT' and 'float'

This looks like it could be related to the container changes in this PR?

@jpkneller
Copy link
Copy Markdown
Contributor

Sorry for the delay! I didn’t get to this before the conference, then spent most of last week ill with some conference virus … 🤒

Aside from the separate discussion about nbsphinx_allow_errors above, I think there’s one more open issue here: In addition to the more exotic model notebooks, which we’ll leave for separate PRs, we also have FlavorTransformation.ipynb failing in the integration tests, with the following error:

TypeError                                 Traceback (most recent call last)
Cell In[6], line 1
----> 1 fig = plot_total_flux(model, AdiabaticMSW(MixingParameters()), AdiabaticMSW(MixingParameters('INVERTED')))
      2 fig.show()
      3 # fig.savefig('flux_adiabaticmsw.pdf')

Cell In[4], line 37
     35 for flavor in Flavor:
     36     for j, E in enumerate(energies):
---> 37         ispec[flavor][j] /= (4.*np.pi*d**2)
     38         ospec_nmo[flavor][j] /= (4.*np.pi*d**2)
     39         ospec_imo[flavor][j] /= (4.*np.pi*d**2)

TypeError: unsupported operand type(s) for /: 'd2NdEdT' and 'float'

This looks like it could be related to the container changes in this PR?

This is an easy fix: look in #408. Change it to

ispec = model.get_flux(times, energies, d)
ospec_nmo = model.get_flux(times, energies, d, xform_nmo)
ospec_imo = model.get_flux(times, energies, d, xform_imo)

Copy link
Copy Markdown
Member

@JostMigenda JostMigenda left a comment

Choose a reason for hiding this comment

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

Looks good to me now; many thanks!

Some of the unit test runs have failed due to what looks like a one-off network issue. I just re-started them and will merge this PR as soon as they turn green.

@JostMigenda JostMigenda merged commit 1eebc63 into main Oct 7, 2025
18 of 30 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.

Review & Update the usage examples' notebooks

3 participants