Skip to content
Open
13 changes: 13 additions & 0 deletions include/AT_StoppingPower.h
Original file line number Diff line number Diff line change
Expand Up @@ -273,4 +273,17 @@ double AT_Mass_Stopping_Power_E_MeV_u_single( const long stopping_power_source_n
const double E_MeV_u, const long particle_no,
const long material_no);

/**
* Checks whether stopping-power data are available for a source,
* particle, and material combination.
*
* @param[in] stopping_power_source_no stopping-power data-source id
* @param[in] particle_no particle number
* @param[in] material_no material number
* @return 1 if data are available, or 0 otherwise
*/
int AT_Stopping_Power_has_data(const long stopping_power_source_no,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please declare the source-based min/max here, next to has_data:

/**
 * Returns the lowest kinetic energy in MeV/u for which the source
 * has stopping-power data for the given particle and material.
 *
 * @param[in]  stopping_power_source_no  id of the data source
 * @param[in]  particle_no               particle number
 * @param[in]  material_no               material number
 * @return     minimum kinetic energy in MeV/u, or -1.0 if there are no data
 */
double AT_Stopping_Power_E_MeV_u_min( const long stopping_power_source_no,
		const long particle_no,
		const long material_no);

/**
 * Returns the highest kinetic energy in MeV/u for which the source
 * has stopping-power data for the given particle and material.
 * For Bethe there is no upper limit and HUGE_VAL is returned.
 *
 * (same parameters)
 * @return     maximum kinetic energy in MeV/u, or -1.0 if there are no data
 */
double AT_Stopping_Power_E_MeV_u_max( const long stopping_power_source_no,
		const long particle_no,
		const long material_no);

Why: this header is the public stopping-power API. With these functions, pyamtrack (and the JS package) needs only this header and the source number it already passes to AT_Mass_Stopping_Power_with_no. At the moment pyamtrack has to include the ICRU and PSTAR data headers and pick the table-specific function itself.

Please also say in the has_data doc that FromFile always returns 0. Otherwise that is a surprise for callers.

const long particle_no,
const long material_no);

#endif /* AT_STOPPINGPOWER_H_ */
19 changes: 19 additions & 0 deletions include/AT_StoppingPowerDataICRU.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,25 @@ int AT_ICRU_wrapper( const long n,
const char info[],
double mass_stopping_power_MeV_cm2_g[]);

/**
* @brief Returns the minimum kinetic energy covered by the ICRU data.
*
* @param[in] material_no Material number identifying the material.
* @param[in] particle_no Particle number identifying the particle.
* @return Minimum tabulated kinetic energy in MeV/u, or -1.0 if
* material_no or particle_no has no ICRU data.
*/
double AT_ICRU_min_energy_MeV_u(long material_no, long particle_no);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as for PSTAR: (particle_no, material_no) order and E_MeV_u_min/max names. I'd also declare the "has data" check and the He limit here:

/** ICRU 49 data for helium end at 250 MeV/u (1000 MeV for He-4) */
#define ICRU_HELIUM_MAX_E_MEV_U   250.0

int AT_ICRU_has_data( const long particle_no,
		const long material_no);
double AT_ICRU_E_MeV_u_min( const long particle_no,
		const long material_no);
double AT_ICRU_E_MeV_u_max( const long particle_no,
		const long material_no);

Why: the constant replaces the two hard-coded 250s (see the comments in the .c file). AT_ICRU_has_data has to be public so that AT_Stopping_Power_has_data can call it directly, instead of inferring availability from min >= 0.


/**
* @brief Returns the maximum kinetic energy covered by the ICRU data.
*
* @param[in] material_no Material number identifying the material.
* @param[in] particle_no Particle number identifying the particle.
* @return Maximum tabulated kinetic energy in MeV/u, or -1.0 if
* material_no or particle_no has no ICRU data.
*/
double AT_ICRU_max_energy_MeV_u(long material_no, long particle_no);

/**
* @struct AT_stopping_power_ICRU_table
Expand Down
19 changes: 19 additions & 0 deletions include/AT_StoppingPowerDataPSTAR.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,25 @@ int AT_PSTAR_wrapper( const long n,
const char info[],
double mass_stopping_power_MeV_cm2_g[]);


/**
* @brief Returns the minimum kinetic energy covered by the PSTAR data.
*
* @param[in] material_no Material number identifying the material.
* @return Minimum tabulated kinetic energy in MeV/u, or -1.0 if
* material_no is outside the PSTAR material range.
*/
double AT_PSTAR_min_energy_MeV_u(long material_no);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please give the PSTAR and ICRU functions the same signature, with particle_no first:

int AT_PSTAR_has_data( const long particle_no,
		const long material_no);
double AT_PSTAR_E_MeV_u_min( const long particle_no,
		const long material_no);
double AT_PSTAR_E_MeV_u_max( const long particle_no,
		const long material_no);

Why:

  • (particle_no, material_no) is the order of every other stopping-power function (AT_PSTAR_wrapper, AT_Mass_Stopping_Power_E_MeV_u_single, ...). The ICRU functions in this PR have it reversed, and since both are long the compiler cannot catch a swapped call.
  • The PSTAR bounds don't depend on the particle, but taking particle_no lets has_data reject invalid particle numbers, and lets AT_Stopping_Power_E_MeV_u_min/max call both tables the same way.
  • E_MeV_u_min/max matches the names of the source-based functions (AT_Stopping_Power_E_MeV_u_min/max).

In the docs, please say that the tables are in MeV for protons and that AT_PSTAR_wrapper looks them up at the proton energy for the same E_MeV_u. That is why the MeV/u limit is the same for all particles, which is not obvious from the code.


/**
* @brief Returns the maximum kinetic energy covered by the PSTAR data.
*
* @param[in] material_no Material number identifying the material.
* @return Maximum tabulated kinetic energy in MeV/u, or -1.0 if
* material_no is outside the PSTAR material range.
*/
double AT_PSTAR_max_energy_MeV_u(long material_no);

/** PSTAR data downloaded from NIST database: http://www.nist.gov/pml/data/star/index.cfm
* Stopping-Power and Range Tables
* for Electrons, Protons, and Helium Ions
Expand Down
32 changes: 32 additions & 0 deletions src/AT_StoppingPower.c
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,35 @@ double AT_Mass_Stopping_Power_E_MeV_u_single( const long stopping_power_source_n
&stopping_power_MeV_cm2_g);
return stopping_power_MeV_cm2_g;
}

int AT_Stopping_Power_has_data(const long stopping_power_source_no,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd make has_data a pure dispatcher to the per-table checks, and add the two min/max functions after it:

int AT_Stopping_Power_has_data( const long stopping_power_source_no,
		const long particle_no,
		const long material_no){

	switch(stopping_power_source_no){
	case Bethe:
		/* analytical formula: needs a valid particle and a known material */
		return AT_Z_from_particle_no_single(particle_no) >= 1 &&
				AT_material_index_from_material_number(material_no) >= 0;
	case PSTAR:
		return AT_PSTAR_has_data(particle_no, material_no);
	case ICRU:
		return AT_ICRU_has_data(particle_no, material_no);
	default:
		/* FromFile: no file name is given here, so availability cannot be checked */
		return 0;
	}
}

double AT_Stopping_Power_E_MeV_u_min( const long stopping_power_source_no,
		const long particle_no,
		const long material_no){

	switch(stopping_power_source_no){
	case Bethe:
		if(!AT_Stopping_Power_has_data(Bethe, particle_no, material_no)){
			return -1.0;
		}
		/* below this limit AT_Bethe_wrapper returns zero */
		return BETHE_LOWER_LIMIT_E_MEV_U;
	case PSTAR:
		return AT_PSTAR_E_MeV_u_min(particle_no, material_no);
	case ICRU:
		return AT_ICRU_E_MeV_u_min(particle_no, material_no);
	default:
		return -1.0;
	}
}

double AT_Stopping_Power_E_MeV_u_max( const long stopping_power_source_no,
		const long particle_no,
		const long material_no){

	switch(stopping_power_source_no){
	case Bethe:
		if(!AT_Stopping_Power_has_data(Bethe, particle_no, material_no)){
			return -1.0;
		}
		/* analytical formula, no upper limit */
		return HUGE_VAL;
	case PSTAR:
		return AT_PSTAR_E_MeV_u_max(particle_no, material_no);
	case ICRU:
		return AT_ICRU_E_MeV_u_max(particle_no, material_no);
	default:
		return -1.0;
	}
}

Why: each table's rules (materials, Z range, energy range) then live only in that table's file, and this file just picks the table by source number. That is the same pattern AT_Mass_Stopping_Power_with_no already uses for the stopping power itself. It also replaces the AT_ICRU_min_energy_MeV_u(...) >= 0.0 check below with a direct AT_ICRU_has_data call, which says what it means.

The A >= 1 check is dropped because none of the three sources use A. The PSTAR and ICRU wrappers take E_MeV_u and use only Z.

const long particle_no,
const long material_no)
{
switch (stopping_power_source_no) {
case FromFile:
/*
* The filename and file contents are not provided here,
* so availability cannot be checked.
*/
return 0;
case Bethe:
/* Bethe is an analytical model, not a finite data table. */
return 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

return 1 for any input means has_data(Bethe, 0, 999) is 1. But Bethe needs a valid particle (it uses Z for the effective charge) and a material from the material table (it reads the average Z and A from it). With material 999, AT_Mass_Stopping_Power_E_MeV_u_single(Bethe, 10.0, 1001, 999) aborts on assert( A > 0 ) in a debug build.

Why it matters: callers such as pyamtrack use has_data to decide whether a call can succeed. A clear 0 is better than 1 followed by an abort. The code in the comment above checks both the particle and the material.

case PSTAR:
if (AT_Z_from_particle_no_single(particle_no) < 1 ||
AT_A_from_particle_no_single(particle_no) < 1) {
return 0;
}
return material_no >= Water_Liquid &&
material_no <= Copper;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This repeats the list of PSTAR materials from AT_StoppingPowerDataPSTAR.c.

Why it's a problem: if a PSTAR table is added, or a slot is left NULL, this range and the actual data disagree, and nothing fails at compile time. AT_PSTAR_has_data(particle_no, material_no) looks at the table itself, so the list stays in one place.

case ICRU:
if (AT_Z_from_particle_no_single(particle_no) < 1 ||
AT_A_from_particle_no_single(particle_no) < 1) {
return 0;
}
return AT_ICRU_min_energy_MeV_u(material_no, particle_no) >= 0.0;
default:
return 0;
}
}
68 changes: 67 additions & 1 deletion src/AT_StoppingPowerDataICRU.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*/

#include "AT_StoppingPowerDataICRU.h"
#include "AT_Error.h"

AT_stopping_power_ICRU_table_struct AT_stopping_power_ICRU_table[2] = {
{ Water_Liquid,
Expand Down Expand Up @@ -128,6 +129,24 @@ AT_stopping_power_ICRU_table_struct AT_stopping_power_ICRU_table[2] = {



static long AT_ICRU_max_atomic_number(const long material_no) {
if (material_no == Water_Liquid) {
return 18;
}
if (material_no == Aluminum_Oxide) {
return 10;
}
return 0;
}

static int AT_ICRU_has_data_for_atomic_number(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The per-material Z limit is the right fix: Ar-40 in Al2O3 now gets -1 instead of a stopping power of 0. I'd turn this into the public AT_ICRU_has_data, taking the particle number:

int AT_ICRU_has_data( const long particle_no,
		const long material_no){
	const long Z = AT_Z_from_particle_no_single(particle_no);
	return Z >= 1 && Z <= AT_ICRU_max_atomic_number(material_no);
}

Why: then "does ICRU have data for this particle in this material?" is answered by exactly one function, used by AT_ICRU_wrapper, the min/max functions and AT_Stopping_Power_has_data. Since AT_ICRU_max_atomic_number returns 0 for any other material, the material check comes for free. The wrapper's (material_no != Water_Liquid) && (material_no != Aluminum_Oxide) can become AT_ICRU_max_atomic_number(material_no) == 0, so the list of ICRU materials is also written only once.

const long material_no,
const long atomic_number) {
const long max_atomic_number =
AT_ICRU_max_atomic_number(material_no);
return atomic_number >= 1 && atomic_number <= max_atomic_number;
}

int AT_ICRU_wrapper( const long n,
const double E_MeV_u[],
const long particle_no[],
Expand All @@ -148,7 +167,7 @@ int AT_ICRU_wrapper( const long n,

for(i = 0; i < n; i++){

if( Z[i] > 18){ /* Data for H ... Ar */
if(!AT_ICRU_has_data_for_atomic_number(material_no, Z[i])){

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With the functions above, the three checks in the wrapper (this one, plus the hard-coded 0.025/1000 and Z[i] == 2 && E_MeV_u[i] > 250 just below) become:

		if(!AT_ICRU_has_data(particle_no[i], material_no)){
			mass_stopping_power_MeV_cm2_g[i]	= -1.0;
			continue;
		}

		if (E_MeV_u[i] < AT_ICRU_E_MeV_u_min(particle_no[i], material_no) ||
				E_MeV_u[i] > AT_ICRU_E_MeV_u_max(particle_no[i], material_no)){
			mass_stopping_power_MeV_cm2_g[i]	= -1.0;
			continue;
		}

Why: the wrapper and the min/max functions must agree on the range. Otherwise a caller who checks E <= max can still get -1.0 from the wrapper. Using the same functions guarantees they agree. I checked that the results don't change at 0.02, 0.025, 1000 and 1000.1 MeV/u for protons, or at 250 and 250.1 MeV/u for He-4.

mass_stopping_power_MeV_cm2_g[i] = -1.0;
continue;
}
Expand Down Expand Up @@ -178,3 +197,50 @@ int AT_ICRU_wrapper( const long n,
return AT_Success;
}

static int AT_ICRU_energy_bounds(long material_no,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With AT_ICRU_has_data, this helper and the two thin wrappers below can become two direct functions, with no out-pointers and no unused locals:

double AT_ICRU_E_MeV_u_min( const long particle_no,
		const long material_no){
	if(!AT_ICRU_has_data(particle_no, material_no)){
		return -1.0;
	}
	return AT_stopping_power_ICRU_table[material_no-1].energy_and_stopping_power[0][0];
}

double AT_ICRU_E_MeV_u_max( const long particle_no,
		const long material_no){
	if(!AT_ICRU_has_data(particle_no, material_no)){
		return -1.0;
	}
	if(AT_Z_from_particle_no_single(particle_no) == 2){
		return ICRU_HELIUM_MAX_E_MEV_U;
	}
	const AT_stopping_power_ICRU_table_struct * table = &AT_stopping_power_ICRU_table[material_no-1];
	return table->energy_and_stopping_power[0][table->number_of_data_points - 1];
}

Why: half the code, and each check runs once. Also, this block is indented with 4 spaces while the rest of the file uses tabs; please use tabs.

long particle_no,
double *minimum,
double *maximum) {
if (material_no != Water_Liquid &&
material_no != Aluminum_Oxide) {
return AT_No_ICRU_Data;
}

long atomic_number = AT_Z_from_particle_no_single(particle_no);
if (!AT_ICRU_has_data_for_atomic_number(material_no, atomic_number)) {
return AT_No_ICRU_Data;
}

const AT_stopping_power_ICRU_table_struct *table =
&AT_stopping_power_ICRU_table[material_no - 1];

*minimum = table->energy_and_stopping_power[0][0];
*maximum = table->energy_and_stopping_power[0]
[table->number_of_data_points - 1];

if (atomic_number == 2) {
*maximum = 250.0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please use the named constant ICRU_HELIUM_MAX_E_MEV_U here (see the header comment).

Why: 250 is currently written both here and in AT_ICRU_wrapper. If someone changes one, the reported max and the wrapper disagree. Once the wrapper calls AT_ICRU_E_MeV_u_max (comment on line 170), the constant is used in exactly one place.

}

return AT_Success;
}

double AT_ICRU_min_energy_MeV_u(long material_no, long particle_no) {
double minimum;
double maximum;
if (AT_ICRU_energy_bounds(material_no, particle_no,
&minimum, &maximum) != AT_Success) {
return -1.0;
}
return minimum;
}

double AT_ICRU_max_energy_MeV_u(long material_no, long particle_no) {
double minimum;
double maximum;
if (AT_ICRU_energy_bounds(material_no, particle_no,
&minimum, &maximum) != AT_Success) {
return -1.0;
}
return maximum;
}
35 changes: 35 additions & 0 deletions src/AT_StoppingPowerDataPSTAR.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "AT_StoppingPowerDataPSTAR.h"
#include "AT_Error.h"

/*
* PSTAR contains experimental stopping power data for protons
Expand Down Expand Up @@ -1341,3 +1342,37 @@ int AT_PSTAR_wrapper( const long n,
return AT_Success;
}

static int AT_PSTAR_energy_bounds(long material_no,
double *minimum,
double *maximum) {
if (material_no < 0 || material_no >= N_PSTAR_MATERIALS) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Crash for material 0 (User_Defined_Material). This range check lets material_no = 0 through, but slot 0 of PSTAR_data.stopping_power_source_data is NULL, so the next lines dereference a null pointer. AT_PSTAR_min_energy_MeV_u(0) segfaults (I reproduced it). Commit 46131eb had a table == NULL check; b1df4d1 dropped it.

I'd put the table lookup in one small helper before AT_PSTAR_wrapper:

/* NULL if material_no is out of range or has no PSTAR table (e.g. User_Defined_Material) */
static const PSTAR_data_for_material_struct * AT_PSTAR_table( const long material_no){
	if( material_no < 0 || material_no >= N_PSTAR_MATERIALS ){
		return NULL;
	}
	return PSTAR_data.stopping_power_source_data[material_no];
}

and use it in the wrapper too:

-	const PSTAR_data_for_material_struct * source_for_given_material = PSTAR_data.stopping_power_source_data[material_no];
+	const PSTAR_data_for_material_struct * source_for_given_material = AT_PSTAR_table(material_no);

Why the wrapper as well: it indexes stopping_power_source_data[material_no] with no range check, so e.g. Tungsten (10) reads past the end of the array. That bug is older than this PR, but with the helper both problems are fixed in one place: the wrapper then returns AT_No_PSTAR_Data for material 10.

return AT_No_PSTAR_Data;
}

*minimum =
PSTAR_data.stopping_power_source_data[material_no]
->energy_and_stopping_power[0][0];
*maximum =
PSTAR_data.stopping_power_source_data[material_no]
->energy_and_stopping_power[N_PSTAR_DATAPOINTS - 1][0];

return AT_Success;
}

double AT_PSTAR_min_energy_MeV_u(long material_no) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With AT_PSTAR_table, the private AT_PSTAR_energy_bounds helper and its out-pointers are not needed. Each function checks once and returns:

int AT_PSTAR_has_data( const long particle_no,
		const long material_no){
	return AT_Z_from_particle_no_single(particle_no) >= 1 &&
			AT_PSTAR_table(material_no) != NULL;
}

double AT_PSTAR_E_MeV_u_min( const long particle_no,
		const long material_no){
	if(!AT_PSTAR_has_data(particle_no, material_no)){
		return -1.0;
	}
	return AT_E_MeV_u_from_E_MeV(AT_PSTAR_table(material_no)->energy_and_stopping_power[0][0],
			PARTICLE_PROTON_NUMBER);
}

double AT_PSTAR_E_MeV_u_max( const long particle_no,
		const long material_no){
	if(!AT_PSTAR_has_data(particle_no, material_no)){
		return -1.0;
	}
	return AT_E_MeV_u_from_E_MeV(AT_PSTAR_table(material_no)->energy_and_stopping_power[N_PSTAR_DATAPOINTS - 1][0],
			PARTICLE_PROTON_NUMBER);
}

Why:

  • AT_PSTAR_has_data becomes the only place that decides whether PSTAR has data. AT_Stopping_Power_has_data can call it instead of repeating the material range (see the comment there).
  • No out-pointers means no unused minimum/maximum locals and nothing left uninitialised on the error path.

The MeV to MeV/u conversion itself is right: the max is 9921.62 MeV/u, which the wrapper turns back into exactly the 10000 MeV table end.

double minimum;
double maximum;
if (AT_PSTAR_energy_bounds(material_no, &minimum, &maximum) != AT_Success) {
return -1.0;
}
return AT_E_MeV_u_from_E_MeV(minimum, PARTICLE_PROTON_NUMBER);
}

double AT_PSTAR_max_energy_MeV_u(long material_no) {
double minimum;
double maximum;
if (AT_PSTAR_energy_bounds(material_no, &minimum, &maximum) != AT_Success) {
return -1.0;
}
return AT_E_MeV_u_from_E_MeV(maximum, PARTICLE_PROTON_NUMBER);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add a newline at the end of the file. Without it, the next change to this file shows this line as modified too.

Loading