Pre SN model related changes - doc, unit tests, Pre SN integration test #66
Conversation
…or presn model use.
…t given the new support for presn models.
…ux files from a snewpy repo.
JostMigenda
left a comment
JostMigenda
left a comment
There was a problem hiding this comment.
This looks good overall; just a few more minor comments.
I don’t like how the hard-coded list of presn model formats is repeated half a dozen times across multiple files; if we ever add another presn model, that’ll be a lot of work to update (and make it too easy to miss on of the locations). That said, given how rare it is to add a new presn models, I’m okay with leaving that as is for now (and cleaning it up later once the actual work for this feature is done).
Regarding the failing tests, those are because we currently require the custom snewpy build. I assume you tried them locally and checked that they run successfully?
doc/documentation.tex
Outdated
| sntools is a Monte Carlo event generator for supernova and pre supernova neutrino interactions. | ||
|
|
||
| It is a command line application that uses detailed time- and energy-dependent neutrino fluxes (provided by various supernova models) to generate interactions within the detector volume and write them to event files that can be used as an input for a full detector simulation. | ||
| sntools was originally developed for Hyper-Kamiokande~\cite{Migenda2019} and later extended to support different detectors and detector materials. | ||
| sntools was originally developed for Hyper-Kamiokande~\cite{Migenda2019} using core collapse supernova models and later extended to support different detectors, detector materials and to use pre supernova models. |
There was a problem hiding this comment.
| sntools is a Monte Carlo event generator for supernova and pre supernova neutrino interactions. | |
| It is a command line application that uses detailed time- and energy-dependent neutrino fluxes (provided by various supernova models) to generate interactions within the detector volume and write them to event files that can be used as an input for a full detector simulation. | |
| sntools was originally developed for Hyper-Kamiokande~\cite{Migenda2019} and later extended to support different detectors and detector materials. | |
| sntools was originally developed for Hyper-Kamiokande~\cite{Migenda2019} using core collapse supernova models and later extended to support different detectors, detector materials and to use pre supernova models. | |
| sntools is a Monte Carlo event generator for supernova and pre-supernova neutrino interactions. | |
| It is a command line application that uses detailed time- and energy-dependent neutrino fluxes (provided by various supernova models) to generate interactions within the detector volume and write them to event files that can be used as an input for a full detector simulation. | |
| sntools was originally developed for Hyper-Kamiokande~\cite{Migenda2019} using core-collapse supernova models and later extended to support different detectors, detector materials and to use pre-supernova models. |
“pre-supernova” is usually hyphenated. I won’t comment on every single instance below, but could you run a Replace All in your editor?
“core-collapse” is less clear. My understanding is that it’s usually hyphenated when used as a composite adjective (as in “a core-collapse supernova”), but not hyphenated when used as a noun (“the core collapse marks the beginning of the explosion”); so that’s the rule I’ve followed in my thesis and would also like to follow here in the sntools docs.
doc/documentation.tex
Outdated
| During the development of support for pre supernova models, a study was performed to optimse the time bin size for Hyper-Kamiokande. The study concluded that a bin size of \SI{1}{s} was the best choice, | ||
| limiting any artificial step features in the distribution of events arising from the nature of the time binning. As a result, \SI{1}{s} is chosen as the default value of this argument. | ||
|
|
||
| The study as a whole suggested an absolute lower limit on the bin size of \SI{50}{ms}, arising from statistical considerations, and an upper limit of 300s to preserve the important features in the event distribution in time before core collapse, | ||
| such as the accelerated event rate close to the core collapse of the star. |
There was a problem hiding this comment.
The 50 ms and 300 s are HK-specific; exact numbers will likely be different for a different detector with other statistics. I’d therefore suggest removing them here.
Will NuPhys have proceedings? If so, we could refer to your proceedings here, so people can look up more details of how this time bin optimisation works.
There was a problem hiding this comment.
NuPhys will have proceedings. So when they are ready, we can add in a line to refer to them for more information.
I have tried these tests locally and they all complete without any problems so I do believe the failure we see is down to the requirement for the custom snewpy build. |
doc/documentation.tex
Outdated
| \subsection{Pre Supernova Time Binning}\label{sec:pre-supernova-time-binning} | ||
| As mentioned in section~\ref{sec:more-details}, when using pre supernova models, the size of the time bins used for calculations within SNTools can be specified through the optional argument \texttt{--binsize <value>}. | ||
| During the development of support for pre supernova models, a study was performed to optimse the time bin size for Hyper-Kamiokande. The study concluded that a bin size of \SI{1}{s} was the best choice, | ||
| \subsection{pre-supernova Time Binning}\label{sec:pre-supernova-time-binning} |
There was a problem hiding this comment.
| \subsection{pre-supernova Time Binning}\label{sec:pre-supernova-time-binning} | |
| \subsection{Pre-Supernova Time Binning}\label{sec:pre-supernova-time-binning} |
|
Excellent, thanks! While you’re finishing the validation, I’ll talk to colleagues about releasing a snewpy version with the fixes that I’ve cherry-picked for the custom branch. |
2 participants
Included in this PR are: updates to the documentation to include Pre SN models, updates to a unit test, the removal of an unnecessary optional argument (--mode) introduced while adding in support for Pre SN models and the addition of a Pre SN integration test.