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

Apply code review changes to MCNP files #665

Merged
merged 6 commits into from
Apr 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 16 additions & 0 deletions news/PR-0665.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
**Added:** None

**Changed:** None
- Applied changes from PullRequest-Agent to MCNP related files:
- using std::err for errors
- update to C++11 standards for converting ints to strings
- removed unnecessary comments
- moved Graveyard and Vacuum strings to variables

**Deprecated:** None

**Removed:** None

**Fixed:** None

**Security:** None
2 changes: 0 additions & 2 deletions src/mcnp/mcnp5/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ set(CMAKE_Fortran_FLAGS_RELWITHDEBINFO "-O1 -g")

# C compiler flags
if (CMAKE_C_COMPILER_ID STREQUAL "Intel")
#set(CMAKE_C_FLAGS "${CMAKE_Fortran_FLAGS} -mcmodel=medium")
Copy link
Member

Choose a reason for hiding this comment

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

The reason these commented lines are here is that they appear in the makefiles for native MCNP, but I needed to turn them off. I don't remember why.

Copy link
Contributor Author

@kkiesling kkiesling Apr 1, 2020

Choose a reason for hiding this comment

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

Should they be left in and put in use? Or is it okay to remove these lines?

Copy link
Member

Choose a reason for hiding this comment

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

Ehhhh yeah let's just delete them

set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pc64")
else ()
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -m64")
Expand All @@ -63,7 +62,6 @@ if (CMAKE_Fortran_COMPILER_ID STREQUAL "Intel")
set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -no-vec")
set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -heap-arrays 1024")
set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -traceback")
#set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -mcmodel=medium")
set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -pc64")
set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -r8")
else ()
Expand Down
46 changes: 20 additions & 26 deletions src/mcnp/mcnp_funcs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ static bool visited_surface = false;
static bool use_dist_limit = false;
static double dist_limit; // needs to be thread-local

static std::string graveyard_str = "Graveyard";
static std::string vacuum_str = "Vacuum";


void dagmcinit_(char* cfile, int* clen, // geom
char* ftol, int* ftlen, // faceting tolerance
Expand Down Expand Up @@ -198,36 +201,36 @@ void write_cell_cards(std::ostringstream& lcadfile, const char* mcnp_version_maj
// that material numbers are assigned
mat_num = DMD->volume_material_data_eh[entity];
// if we cant make an int from the mat_num
if (mat_num.find("Graveyard") == std::string::npos &&
mat_num.find("Vacuum") == std::string::npos) {
if (mat_num.find(graveyard_str) == std::string::npos &&
mat_num.find(vacuum_str) == std::string::npos) {
if (!DMD->try_to_make_int(mat_num)) {
std::cout << "Failed to cast material number to an integer" << std::endl;
std::cout << "volume with ID " << cellid << " has material assignment" << std::endl;
std::cout << mat_num << " which appears to be a name rather than an integer" << std::endl;
std::cout << "Did you forget to run uwuw_preproc?" << std::endl;
std::cerr << "Failed to cast material number to an integer" << std::endl;
std::cerr << "volume with ID " << cellid << " has material assignment" << std::endl;
std::cerr << mat_num << " which appears to be a name rather than an integer" << std::endl;
std::cerr << "Did you forget to run uwuw_preproc?" << std::endl;
exit(EXIT_FAILURE);
}
}

density = DMD->volume_density_data_eh[entity];
// if we have a vacuum problem
if (mat_num == "Graveyard" || mat_num == "Vacuum") {
if (mat_num == graveyard_str || mat_num == vacuum_str) {
mat_num = "0";
density = "";
}
} else {
std::string mat_name = DMD->volume_material_property_data_eh[entity];
// if we not vacuum or graveyard
if (mat_name.find("Vacuum") == std::string::npos && mat_name.find("Graveyard") == std::string::npos) {
if (mat_name.find(vacuum_str) == std::string::npos && mat_name.find(graveyard_str) == std::string::npos) {
if (workflow_data->material_library.count(mat_name) == 0) {
std::cout << "Material with name " << mat_name << " not found " << std::endl;
std::cout << "In the material library" << std::endl;
std::cerr << "Material with name " << mat_name << " not found " << std::endl;
std::cerr << "In the material library" << std::endl;
exit(EXIT_FAILURE);
}

pyne::Material material = workflow_data->material_library[mat_name];
int matnumber = material.metadata["mat_number"].asInt();
mat_num = _to_string(matnumber);
mat_num = std::to_string(matnumber);
density = "-" + _to_string(material.density);
} else {
mat_num = "0";
Expand All @@ -251,15 +254,15 @@ void write_cell_cards(std::ostringstream& lcadfile, const char* mcnp_version_maj
} else if (mcnp_version_major[0] == '6') {
mcnp_name = pyne::particle::mcnp6(particle_name);
} else {
std::cout << "Unknown MCNP verison: " << mcnp_version_major << std::endl;
std::cerr << "Unknown MCNP verison: " << mcnp_version_major << std::endl;
exit(EXIT_FAILURE);
}
double imp = 1.0;
// if we find graveyard always have importance 0.0
if (mat_name.find("Graveyard") != std::string::npos) {
if (mat_name.find(graveyard_str) != std::string::npos) {
imp = 0.0;
// no splitting can happenin vacuum set to 1
} else if (mat_name.find("Vacuum") != std::string::npos) {
} else if (mat_name.find(vacuum_str) != std::string::npos) {
imp = 1.0;
// otherwise as the map says
} else {
Expand All @@ -269,15 +272,15 @@ void write_cell_cards(std::ostringstream& lcadfile, const char* mcnp_version_maj
}
// its possible no importances were assigned
if (set.size() == 0) {
if (mat_name.find("Graveyard") == std::string::npos) {
if (mat_name.find(graveyard_str) == std::string::npos) {
importances = "imp:n=1";
} else {
importances = "imp:n=0";
}
}

// add descriptive comments for special volumes
if (mat_name.find("Graveyard") != std::string::npos) {
if (mat_name.find(graveyard_str) != std::string::npos) {
importances += " $ graveyard";
} else if (DAG->is_implicit_complement(entity)) {
importances += " $ implicit complement";
Expand Down Expand Up @@ -307,7 +310,7 @@ void write_surface_cards(std::ostringstream& lcadfile) {
surface_property = "+";
else
surface_property = "";
lcadfile << surface_property << _to_string(surfid) << std::endl;
lcadfile << surface_property << std::to_string(surfid) << std::endl;
}
return;
}
Expand Down Expand Up @@ -787,15 +790,6 @@ void dagmc_teardown_() {
delete DAG;
}

// these functions should be replaced when we adopt C++11
// int to string
std::string _to_string(int var) {
std::ostringstream outstr;
outstr << var;
std::string ret_string = outstr.str();
return ret_string;
}

// double to string
std::string _to_string(double var) {
std::ostringstream outstr;
Expand Down
3 changes: 0 additions & 3 deletions src/mcnp/mcnp_funcs.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,6 @@ void write_surface_cards(std::ostringstream& lcad_string);
void write_material_data(std::ostringstream& lcad_string);
void write_tally_data(std::ostringstream& lcad_string);

// until we adopt C++11 - makes life easy
// convenience functions for c++ int to string
std::string _to_string(int val);
// convenience functions for c++ double to string
std::string _to_string(double val);

Expand Down