Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pulse height tally for photons #2452

Merged
merged 82 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
82 commits
Select commit Hold shift + click to select a range
974a720
pulse-height tally implementation v2.0
Apr 3, 2023
dd18f12
bugfixes
Apr 3, 2023
199e23b
description of the score pulse-height
Apr 3, 2023
38b7f70
type in score dic
Apr 3, 2023
89820f2
remove blank line
Apr 3, 2023
c676ad7
Clang Format
Apr 14, 2023
eeb8a49
Clang format
Apr 14, 2023
82f7b90
clang
Apr 14, 2023
11a7ed2
clang format - final
Apr 14, 2023
7c2142e
clang
Apr 14, 2023
3ce4b9c
adding global variable pulse_height_cells
Apr 27, 2023
7cdbada
changing the value in the pht_storage at index
Apr 27, 2023
7c9c991
calling the right index of the pht_storage
Apr 27, 2023
01e3506
small changes
Apr 27, 2023
a746d37
Merge branch 'develop' into pulse-height-2
Apr 30, 2023
158c3af
update
Apr 30, 2023
d32174e
addressing issue #2508
May 1, 2023
09de63d
missing break statement in tally.cpp
May 1, 2023
e401990
revert last changes
May 2, 2023
78bd621
Merge branch 'develop' into pulse-height-2
May 2, 2023
deb4196
small change comment
May 2, 2023
1712d5c
p.alive in process_advance_particle_events
May 3, 2023
7090c5c
redo last change
May 3, 2023
7c7c0a4
undo last change
May 3, 2023
2626347
Merge branch 'pulse-height-2' of https://github.com/cfichtlscherer/op…
May 3, 2023
fcebe5c
run again with sort surf bank
May 4, 2023
74a1640
added regression test
May 4, 2023
795145a
reset pht storage for event based simulations
May 4, 2023
7516002
remove sorting surface source
May 5, 2023
efc476f
Update docs/source/usersguide/tallies.rst
cfichtlscherer May 25, 2023
6873cc4
Update include/openmc/particle_data.h
cfichtlscherer May 25, 2023
91f561a
Update include/openmc/particle_data.h
cfichtlscherer May 25, 2023
c34aa0c
Update src/particle.cpp
cfichtlscherer May 25, 2023
32714aa
Update src/particle_data.cpp
cfichtlscherer May 25, 2023
31e47b1
Update src/tallies/tally.cpp
cfichtlscherer May 25, 2023
1e0365f
description tallies.cpp
May 25, 2023
1d69270
clang
May 25, 2023
9274af0
description particle.cpp
May 25, 2023
f42d316
easyier check if cell index in pulse_height_cells
May 25, 2023
1dcb51e
clang
May 25, 2023
6d4cfee
Update src/tallies/tally.cpp
cfichtlscherer May 25, 2023
27692b5
missing semicolon
May 25, 2023
538a06e
include container_util
May 25, 2023
d7423fb
missing if
May 25, 2023
d1a61fa
clang
May 25, 2023
62f8c58
description tallies
May 25, 2023
8ff651e
update pulse-height test
May 25, 2023
a727c4c
checking only cell / energy filter
May 25, 2023
36172d3
clang
May 25, 2023
464e504
adding pytest pht test
May 25, 2023
8e260d9
include cellborn filter
May 25, 2023
ac2e6e2
update pulse-height score function
May 25, 2023
4b9f2e1
clang
May 25, 2023
11ec770
update scoring function
May 25, 2023
dbf6aa4
clang
May 25, 2023
7967e8f
type
May 25, 2023
57f1425
box -> sphere pht test
May 25, 2023
6b7bcf0
update
Jun 2, 2023
d689d91
clang
Jun 2, 2023
8fab57a
remove estimator
cfichtlscherer Jun 8, 2023
fb547a9
remove estimator py / small changes
cfichtlscherer Jun 8, 2023
16cbc2a
when score check if cell in pht_cells
cfichtlscherer Jun 8, 2023
7c0804a
clang
cfichtlscherer Jun 8, 2023
48d2277
iterate over all cells in all cell filters
cfichtlscherer Jun 11, 2023
cfd219a
update test
cfichtlscherer Jun 11, 2023
7aa7c4f
comment photon transport / clang
cfichtlscherer Jun 20, 2023
969ef10
clang update
cfichtlscherer Jun 20, 2023
63db229
update test
cfichtlscherer Jun 20, 2023
a630e89
cell filter check
cfichtlscherer Jun 20, 2023
0bf609d
asking birth cell earlier
cfichtlscherer Jun 20, 2023
e8a412d
clang
cfichtlscherer Jun 20, 2023
71b0e17
clang
cfichtlscherer Jun 20, 2023
3eb998f
clang
cfichtlscherer Jun 21, 2023
ff0f6db
clang
cfichtlscherer Jun 21, 2023
e4b574d
clang old version
cfichtlscherer Jun 21, 2023
6f7d8ad
clange
cfichtlscherer Jun 23, 2023
01a03a6
test clang
cfichtlscherer Jun 24, 2023
c796fa1
changes filt->type() in tally
cfichtlscherer Jul 7, 2023
b10a32e
clang
cfichtlscherer Jul 7, 2023
b43ebae
dynamic_cast to FilterType
cfichtlscherer Jul 8, 2023
24d8cb5
type
cfichtlscherer Jul 8, 2023
ebee016
type 2
cfichtlscherer Jul 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/source/usersguide/tallies.rst
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,11 @@ The following tables show all valid scores:
| |particle. This corresponds to MT=444 produced by |
| |NJOY's HEATR module. |
+----------------------+---------------------------------------------------+
|pulse-height |The energy deposited by an entire photons history |
| |(including progenies). Units are eV per source |
cfichtlscherer marked this conversation as resolved.
Show resolved Hide resolved
| |particle. Note that this score can only be combined|
| |with a cell filter and an energy filter. |
+----------------------+---------------------------------------------------+

.. _usersguide_tally_normalization:

Expand Down
11 changes: 6 additions & 5 deletions include/openmc/constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ constexpr double MASS_NEUTRON_EV {
939.56542052e6}; // mass of a neutron in eV/c^2
constexpr double MASS_PROTON {1.007276466621}; // mass of a proton in amu
constexpr double MASS_ELECTRON_EV {
0.51099895000e6}; // electron mass energy equivalent in eV/c^2
0.51099895000e6}; // electron mass energy equivalent in eV/c^2
constexpr double FINE_STRUCTURE {
137.035999084}; // inverse fine structure constant
137.035999084}; // inverse fine structure constant
constexpr double PLANCK_C {
1.2398419839593942e4}; // Planck's constant times c in eV-Angstroms
constexpr double AMU {1.66053906660e-27}; // 1 amu in kg
Expand Down Expand Up @@ -278,9 +278,9 @@ enum class MgxsType {

enum class TallyResult { VALUE, SUM, SUM_SQ };

enum class TallyType { VOLUME, MESH_SURFACE, SURFACE };
enum class TallyType { VOLUME, MESH_SURFACE, SURFACE, PULSE_HEIGHT };

enum class TallyEstimator { ANALOG, TRACKLENGTH, COLLISION };
enum class TallyEstimator { ANALOG, TRACKLENGTH, COLLISION, PULSE_HEIGHT };

enum class TallyEvent { SURFACE, LATTICE, KILL, SCATTER, ABSORB };

Expand All @@ -306,7 +306,8 @@ enum TallyScore {
SCORE_INVERSE_VELOCITY = -13, // flux-weighted inverse velocity
SCORE_FISS_Q_PROMPT = -14, // prompt fission Q-value
SCORE_FISS_Q_RECOV = -15, // recoverable fission Q-value
SCORE_DECAY_RATE = -16 // delayed neutron precursor decay rate
SCORE_DECAY_RATE = -16, // delayed neutron precursor decay rate
SCORE_PULSE_HEIGHT = -17 // pulse-height
};

// Global tally parameters
Expand Down
2 changes: 1 addition & 1 deletion include/openmc/mcpl_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ vector<SourceSite> mcpl_source_sites(std::string path);
//! calculate_parallel_index_vector on
//! source_bank.size().
void write_mcpl_source_point(const char* filename,
gsl::span<SourceSite> source_bank, vector<int64_t> const& bank_index);
gsl::span<SourceSite> source_bank, const vector<int64_t>& bank_index);
} // namespace openmc

#endif // OPENMC_MCPL_INTERFACE_H
4 changes: 4 additions & 0 deletions include/openmc/particle.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ class Particle : public ParticleData {
void event_revive_from_secondary();
void event_death();

//! pulse-height recording
void pht_collision_energy();
void pht_secondary_particles();

//! Cross a surface and handle boundary conditions
void cross_surface();

Expand Down
23 changes: 13 additions & 10 deletions include/openmc/particle_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ class LocalCoord {

struct NuclideMicroXS {
// Microscopic cross sections in barns
double total; //!< total cross section
double absorption; //!< absorption (disappearance)
double fission; //!< fission
double nu_fission; //!< neutron production from fission
double total; //!< total cross section
double absorption; //!< absorption (disappearance)
double fission; //!< fission
double nu_fission; //!< neutron production from fission

double elastic; //!< If sab_frac is not 1 or 0, then this value is
//!< averaged over bound and non-bound nuclei
Expand Down Expand Up @@ -231,7 +231,7 @@ class ParticleData {
vector<ElementMicroXS> photon_xs_; //!< Microscopic photon cross sections
MacroXS macro_xs_; //!< Macroscopic cross sections

int64_t id_; //!< Unique ID
int64_t id_; //!< Unique ID
ParticleType type_ {ParticleType::neutron}; //!< Particle type (n, p, e, etc.)

int n_coord_ {1}; //!< number of current coordinate levels
Expand Down Expand Up @@ -302,23 +302,25 @@ class ParticleData {
// Secondary particle bank
vector<SourceSite> secondary_bank_;

int64_t current_work_; // current work index
int64_t current_work_; // current work index

vector<double> flux_derivs_; // for derivatives for this particle
vector<double> flux_derivs_; // for derivatives for this particle

vector<FilterMatch> filter_matches_; // tally filter matches

vector<TrackStateHistory> tracks_; // tracks for outputting to file
vector<TrackStateHistory> tracks_; // tracks for outputting to file

vector<NuBank> nu_bank_; // bank of most recently fissioned particles
vector<NuBank> nu_bank_; // bank of most recently fissioned particles

std::vector<double> pht_storage_; // interim pulse-height results
cfichtlscherer marked this conversation as resolved.
Show resolved Hide resolved

// Global tally accumulators
double keff_tally_absorption_ {0.0};
double keff_tally_collision_ {0.0};
double keff_tally_tracklength_ {0.0};
double keff_tally_leakage_ {0.0};

bool trace_ {false}; //!< flag to show debug information
bool trace_ {false}; //!< flag to show debug information

double collision_distance_; // distance to particle's next closest collision

Expand Down Expand Up @@ -443,6 +445,7 @@ class ParticleData {
decltype(tracks_)& tracks() { return tracks_; }
decltype(nu_bank_)& nu_bank() { return nu_bank_; }
NuBank& nu_bank(int i) { return nu_bank_[i]; }
std::vector<double>& pht_storage() { return pht_storage_; }
cfichtlscherer marked this conversation as resolved.
Show resolved Hide resolved

double& keff_tally_absorption() { return keff_tally_absorption_; }
double& keff_tally_collision() { return keff_tally_collision_; }
Expand Down
10 changes: 5 additions & 5 deletions include/openmc/plot.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace model {

extern std::unordered_map<int, int> plot_map; //!< map of plot ids to index
extern vector<std::unique_ptr<PlottableInterface>>
plots; //!< Plot instance container
plots; //!< Plot instance container

extern uint64_t plotter_seed; // Stream index used by the plotter

Expand Down Expand Up @@ -321,11 +321,11 @@ class ProjectionPlot : public PlottableInterface {
// loop:
static const int MAX_INTERSECTIONS = 1000000;

std::array<int, 2> pixels_; // pixel dimension of resulting image
std::array<int, 2> pixels_; // pixel dimension of resulting image
double horizontal_field_of_view_ {70.0}; // horiz. f.o.v. in degrees
Position camera_position_; // where camera is
Position look_at_; // point camera is centered looking at
Direction up_ {0.0, 0.0, 1.0}; // which way is up
Position camera_position_; // where camera is
Position look_at_; // point camera is centered looking at
Direction up_ {0.0, 0.0, 1.0}; // which way is up

// which color IDs should be wireframed. If empty, all cells are wireframed.
vector<int> wireframe_ids_;
Expand Down
8 changes: 6 additions & 2 deletions include/openmc/tallies/tally.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ class Tally {
//----------------------------------------------------------------------------
// Major public data members.

int id_ {C_NONE}; //!< User-defined identifier
int id_ {C_NONE}; //!< User-defined identifier

std::string name_; //!< User-defined name
std::string name_; //!< User-defined name

TallyType type_ {TallyType::VOLUME}; //!< e.g. volume, surface current

Expand Down Expand Up @@ -121,8 +121,10 @@ class Tally {
// We need to have quick access to some filters. The following gives indices
// for various filters that could be in the tally or C_NONE if they are not
// present.
int energy_filter_ {C_NONE};
int energyout_filter_ {C_NONE};
int delayedgroup_filter_ {C_NONE};
int cell_filter_ {C_NONE};

vector<Trigger> triggers_;

Expand Down Expand Up @@ -155,6 +157,8 @@ extern vector<int> active_tracklength_tallies;
extern vector<int> active_collision_tallies;
extern vector<int> active_meshsurf_tallies;
extern vector<int> active_surface_tallies;
extern vector<int> active_pulse_height_tallies;
extern vector<int> pulse_height_cells;
} // namespace model

namespace simulation {
Expand Down
7 changes: 7 additions & 0 deletions include/openmc/tallies/tally_scoring.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,13 @@ void score_tracklength_tally(Particle& p, double distance);
//! \param tallies A vector of tallies to score to
void score_surface_tally(Particle& p, const vector<int>& tallies);

//! Score the pulse-height tally
//! This is triggered at the end of every particle history
//
//! \param p The particle being tracked
//! \param tallies A vector of tallies to score to
cfichtlscherer marked this conversation as resolved.
Show resolved Hide resolved
void score_pulse_height_tally(Particle& p, const vector<int>& tallies);

} // namespace openmc

#endif // OPENMC_TALLIES_TALLY_SCORING_H
8 changes: 4 additions & 4 deletions openmc/lib/tally.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,13 @@
-5: 'absorption', -6: 'fission', -7: 'nu-fission', -8: 'kappa-fission',
-9: 'current', -10: 'events', -11: 'delayed-nu-fission',
-12: 'prompt-nu-fission', -13: 'inverse-velocity', -14: 'fission-q-prompt',
-15: 'fission-q-recoverable', -16: 'decay-rate'
-15: 'fission-q-recoverable', -16: 'decay-rate', -17: 'pulse-height'
}
_ESTIMATORS = {
0: 'analog', 1: 'tracklength', 2: 'collision'
0: 'analog', 1: 'tracklength', 2: 'collision', 3: 'pulse-height'
}
_TALLY_TYPES = {
0: 'volume', 1: 'mesh-surface', 2: 'surface'
0: 'volume', 1: 'mesh-surface', 2: 'surface', 3: 'pulse-height'
}


Expand Down Expand Up @@ -169,7 +169,7 @@ class Tally(_FortranObjectWithID):
id : int
ID of the tally
estimator: str
Estimator type of tally (analog, tracklength, collision)
Estimator type of tally (analog, tracklength, collision, pulse-height)
filters : list
List of tally filters
mean : numpy.ndarray
Expand Down
2 changes: 1 addition & 1 deletion openmc/tallies.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
_FILTER_CLASSES = (openmc.Filter, openmc.CrossFilter, openmc.AggregateFilter)

# Valid types of estimators
ESTIMATOR_TYPES = ['tracklength', 'collision', 'analog']
ESTIMATOR_TYPES = ['tracklength', 'collision', 'analog', 'pulse-height']


class Tally(IDManagerMixin):
Expand Down
15 changes: 9 additions & 6 deletions src/dagmc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,8 @@ std::pair<double, int32_t> DAGCell::distance(
// into the implicit complement on the other side where no intersection will
// be found. Treating this as a lost particle is problematic when plotting.
// Instead, the infinite distance and invalid surface index are returned.
if (settings::run_mode == RunMode::PLOTTING) return {INFTY, -1};
if (settings::run_mode == RunMode::PLOTTING)
return {INFTY, -1};

// the particle should be marked as lost immediately if an intersection
// isn't found in a volume that is not the implicit complement. In the case
Expand Down Expand Up @@ -739,7 +740,8 @@ void check_dagmc_root_univ()
}
}

int32_t next_cell(int32_t surf, int32_t curr_cell, int32_t univ) {
int32_t next_cell(int32_t surf, int32_t curr_cell, int32_t univ)
{
auto surfp = dynamic_cast<DAGSurface*>(model::surfaces[surf - 1].get());
auto cellp = dynamic_cast<DAGCell*>(model::cells[curr_cell].get());
auto univp = static_cast<DAGUniverse*>(model::universes[univ].get());
Expand All @@ -750,11 +752,12 @@ int32_t next_cell(int32_t surf, int32_t curr_cell, int32_t univ) {
cellp->dagmc_ptr()->entity_by_index(3, cellp->dag_index());

moab::EntityHandle new_vol;
moab::ErrorCode rval = cellp->dagmc_ptr()->next_vol(surf_handle, curr_vol, new_vol);
if (rval != moab::MB_SUCCESS) return -1;
moab::ErrorCode rval =
cellp->dagmc_ptr()->next_vol(surf_handle, curr_vol, new_vol);
if (rval != moab::MB_SUCCESS)
return -1;

return cellp->dagmc_ptr()->index_by_handle(new_vol) +
univp->cell_idx_offset_;
return cellp->dagmc_ptr()->index_by_handle(new_vol) + univp->cell_idx_offset_;
}

} // namespace openmc
Expand Down
5 changes: 3 additions & 2 deletions src/output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
#include <cstring> // for strlen
#include <ctime> // for time, localtime
#include <fstream>
#include <iomanip> // for setw, setprecision, put_time
#include <ios> // for fixed, scientific, left
#include <iomanip> // for setw, setprecision, put_time
#include <ios> // for fixed, scientific, left
#include <iostream>
#include <sstream>
#include <unordered_map>
Expand Down Expand Up @@ -594,6 +594,7 @@ const std::unordered_map<int, const char*> score_names = {
{SCORE_FISS_Q_PROMPT, "Prompt fission power"},
{SCORE_FISS_Q_RECOV, "Recoverable fission power"},
{SCORE_CURRENT, "Current"},
{SCORE_PULSE_HEIGHT, "pulse-height"},
};

//! Create an ASCII output file showing all tally results.
Expand Down
59 changes: 58 additions & 1 deletion src/particle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "openmc/message_passing.h"
#include "openmc/mgxs_interface.h"
#include "openmc/nuclide.h"
#include "openmc/particle_data.h"
#include "openmc/photon.h"
#include "openmc/physics.h"
#include "openmc/physics_mg.h"
Expand Down Expand Up @@ -286,6 +287,11 @@ void Particle::event_collide()
}
}

if (!model::active_pulse_height_tallies.empty() &&
type() == ParticleType::photon) {
pht_collision_energy();
}

// Reset banked weight during collision
n_bank() = 0;
n_bank_second() = 0;
Expand Down Expand Up @@ -350,6 +356,11 @@ void Particle::event_revive_from_secondary()
secondary_bank().pop_back();
n_event() = 0;

if (!model::active_pulse_height_tallies.empty() &&
cfichtlscherer marked this conversation as resolved.
Show resolved Hide resolved
this->type() == ParticleType::photon) {
pht_secondary_particles();
}

// Enter new particle in particle track file
if (write_track())
add_particle_track(*this);
Expand Down Expand Up @@ -383,6 +394,10 @@ void Particle::event_death()
keff_tally_tracklength() = 0.0;
keff_tally_leakage() = 0.0;

if (!model::active_pulse_height_tallies.empty()) {
score_pulse_height_tally(*this, model::active_pulse_height_tallies);
}

// Record the number of progeny created by this particle.
// This data will be used to efficiently sort the fission bank.
if (settings::run_mode == RunMode::EIGENVALUE) {
Expand All @@ -391,6 +406,46 @@ void Particle::event_death()
}
}

void Particle::pht_collision_energy()
{
// determine index of cell in pulse_height_cells
auto it = std::find(model::pulse_height_cells.begin(),
model::pulse_height_cells.end(), coord(n_coord() - 1).cell);
int index = std::distance(model::pulse_height_cells.begin(), it);
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if the particle is in a cell that is not listed in pulse_height_cells?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thats an important point! I will just call the action when the cell is listed:

if(it != model::pulse_height_cells.end()){

think this should solve the issue

// Adds the energy particles lose in a collision to the pulse-height at the
// cell index
pht_storage()[index] += E_last() - E();

// If the energy of the particle is below the cutoff, it will not be sampled
// so its energy is added to the pulse-height in the cell
if (E() < settings::energy_cutoff[1]) {
cfichtlscherer marked this conversation as resolved.
Show resolved Hide resolved
pht_storage()[index] += E();
}
}

void Particle::pht_secondary_particles()
{
// Removes the energy of secondary produced particles from the pulse-height

// determine the birth cell of the particle
if (coord(n_coord() - 1).cell == C_NONE) {
if (!exhaustive_find_cell(*this)) {
mark_as_lost(
"Could not find the cell containing particle " + std::to_string(id()));
return;
}

// Set birth cell attribute
if (cell_born() == C_NONE)
cell_born() = coord(n_coord() - 1).cell;
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like it will change the behavior for CellBornFilter? I've always thought of cell born as corresponding to the cell that the original source particle was born in (not the cell a secondary particle was born in), although looking through the code now, I'm not sure that's actually how it's treated at present. Curious to hear your thoughts. Either way, setting the cell_born attribute on a secondary particle probably belongs somewhere else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We had a discussion on this already in the old version of the pht:
https://github.com/openmc-dev/openmc/pull/1881/files#r726887201

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have simulated a simple example (happy to share) and currently the cell_born attribute is not corresponding for the original source particle. If a secondary particle is created in a cell the cell_born attribute is set to this cell for the secondary particle.

Maybe we should add a small comment into the docs?
Instead of
Bins tally events based on which cell the particle was born in.
maybe the following solves some confusion:
Bins tally events based on which cell the particle was born in. For secondary particles this attribute is set to the cell in which they were produced.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is how the cell_born attribute is currently handled in my understanding:

  1. particles start with a None entry.
  2. in the transport_history_based_single_particle function we are calling in the first step of the loop always the p.event_calculate_xs() method. In this method, the cell_born attribute is set.

In the case of the pulse-height tally we have to remove the energy of secondary particles that were created, cause their energy may not remain in the cell. So whenever a particle is created we remove the energy directly from the pulse_height value.
Since this attribute is set from a None to the cell the particle was born in only a bit earlier, this will not change the behaviour of the CellBornFilter.
I see that this is not really elegant. But changing it would require a method to distinguish whether a particle is a primary or a secondary particle. Therefore I would recommend to leave it as it is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, sorry for my misunderstanding. Now I see that this code is actually exactly what's being called in event_calculate_xs, so no behavior is changing. That being said, it still feels awkward to me that this block of code is called in pht_secondary_particles. Can you move it up into event_revive_from_secondary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, so of course it would be best if we could design the code in a way that the birth cell needs to be determined only in one place.
But, as said, I think this is hardly doable. Since not all particles start from event_revive_from_secondary(). So my conclusion from this is that we need to determine it separately for the secondary particles for the pulse-height tally.

On the other hand, it is not necessary to determine the birth_cell for all secondary particles. We have to do this only if the information is needed for the pulse-height tally.

Coming from that argumentation, I set the birth_cell now in the if condition just before the pulse-height function is called.

So the birth cell is just determined before the

if (!model::active_pulse_height_tallies.empty() &&
        this->type() == ParticleType::photon) {

condition.

}
// determine index of cell in pulse_height_cells
auto it = std::find(model::pulse_height_cells.begin(),
model::pulse_height_cells.end(), cell_born());
int index = std::distance(model::pulse_height_cells.begin(), it);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question here -- seems like this could result in out-of-bounds access

pht_storage()[index] -= E();
}

void Particle::cross_surface()
{
int i_surface = std::abs(surface());
Expand Down Expand Up @@ -433,7 +488,9 @@ void Particle::cross_surface()
#ifdef DAGMC
// in DAGMC, we know what the next cell should be
if (surf->geom_type_ == GeometryType::DAG) {
int32_t i_cell = next_cell(i_surface, cell_last(n_coord() - 1), lowest_coord().universe) - 1;
int32_t i_cell =
next_cell(i_surface, cell_last(n_coord() - 1), lowest_coord().universe) -
1;
// save material and temp
material_last() = material();
sqrtkT_last() = sqrtkT();
Expand Down
Loading