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

Adding console output methods to some classes #872

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 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
2 changes: 0 additions & 2 deletions .github/workflows/linux_build_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ jobs:
steps:
- name: Checkout repository

Choose a reason for hiding this comment

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

I think this may fix the linux build test failure

Suggested change
- name: Checkout repository
- name: Add exception for dubious ownership
run: git config --global --add safe.directory /__w/DAGMC/DAGMC
- name: Checkout repository

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, but I think this PR has been replaced by another ( #876 ), so will be close by @pshriwise soon.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct! Closing now.

uses: actions/checkout@v3
with:
submodules: recursive

- name: Setup
run: |
Expand Down
2 changes: 0 additions & 2 deletions .github/workflows/linux_build_test_merge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ jobs:
steps:
- name: Checkout repository
uses: actions/checkout@v3
with:
submodules: recursive

- name: Setup
run: |
Expand Down
6 changes: 3 additions & 3 deletions doc/CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ Next version
* Removed unused Circle CI yml (#859)
* Added configuration options to CMake configuration file (#867)
* Change test-on-merge against MOAB master/develop to be optional (#870)
* Enhance console logging capabilities (#872)

**Fixed:**
* Patch to compile with Geant4 10.6

* Patch to compile with Geant4 10.6

v3.2.2
====================
Expand All @@ -48,7 +48,7 @@ v3.2.1
**Removed:**

**Fixed:**

**Security:**

**Maintenance:**
Expand Down
53 changes: 53 additions & 0 deletions src/dagmc/DagMC.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,56 @@ class DagMC {
return MBI_shared_ptr;
}

// Verbosity levels
// -1 : "override" the current verbosity setting and write message
// 0 : No output
// 1 : All output
int set_verbosity(int val) {
int verbosity_max = 0;
int verbosity_min = 1;
if (val < verbosity_min || val > verbosity_max)
warning("Invalid verbosity value " + std::to_string(val) +
" will be set to nearest valid value.");
val = std::min(std::max(verbosity_min, val), verbosity_max);
return verbosity = val;
Copy link
Member

Choose a reason for hiding this comment

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

Unless my brain is missing something, this will always result in 0?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mm yeah mentally bit-flipped the initializer values at the beginning of this function.

}

int get_verbosity(int val) { return verbosity; }
Copy link
Member

Choose a reason for hiding this comment

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

Why does this take an argument- seems confusing


/** console output mechanism
* @param msg Message to be written to the console
* @param lvl Message will not be written if the verbosity level
* is higher than the class instance's current verbosity setting.
* Use of -1 ensures the message will always be written.
* @param newline Whether or not to apend a newline to the message.
*/
void message(const std::string& msg, int lvl = 1, bool newline = true) const {
// do not write message if level is higher than
// verbosity setting
if (lvl > verbosity) return;
// otherwise, display message
if (newline)
std::cout << msg << "\n";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
std::cout << msg << "\n";
std::cout << msg << std::endl;

Copy link
Member Author

@pshriwise pshriwise Apr 4, 2023

Choose a reason for hiding this comment

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

I think it's preferred to use a newline literal these days. Using std::endl will force the output buffer to flush and is more likely to cause garbled messages in concurrent applications, so I thought I'd start taking us in that direction.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could do it all in one go in a separate changeset though.

Copy link
Member

Choose a reason for hiding this comment

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

I thought it was preferred because it is platform independent (e.g. Windows carriage returns...). That's the only reason I suggested it.

else
std::cout << msg;
}

/** write a warning message to the console
* @param msg Message to be written to the console
* @param newline Whether or not to apend a newline to the message.
*/
void warning(const std::string& msg, bool newline = true) const {
message("WARNING: " + msg, -1, newline);
}

/** write an error message to the console
* @param msg Message to be written to the console
* @param newline Whether or not to apend a newline to the message.
*/
void err_msg(const std::string& msg, bool newline = true) const {
message("ERROR: " + msg, -1, newline);
}

private:
/* PRIVATE MEMBER DATA */

Expand All @@ -486,6 +536,9 @@ class DagMC {

std::unique_ptr<RayTracer> ray_tracer;

/** verbosity setting for DAGMC operations */
int verbosity{1};

public:
Tag nameTag, facetingTolTag;

Expand Down
51 changes: 28 additions & 23 deletions src/dagmc/dagmcmetadata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
#include <algorithm>
#include <iostream>
#include <set>
#include <sstream>

#include "util.hpp"

// constructor for metadata class
dagmcMetaData::dagmcMetaData(moab::DagMC* dag_ptr, bool verbosity,
bool require_density_present)
: DAG(dag_ptr),
verbose(verbosity),
verbosity(verbosity ? 1 : 0),
require_density(require_density_present) {
// these are the keywords that dagmc will understand
// from groups if you need to process more
Expand Down Expand Up @@ -50,7 +51,7 @@ std::string dagmcMetaData::get_volume_property(std::string property,
} else if (property == "tally") {
value = tally_data_eh[eh];
} else {
std::cout << "Not a valid property for volumes" << std::endl;
std::cerr << "Not a valid property for volumes" << std::endl;
}
return value;
}
Expand Down Expand Up @@ -228,7 +229,7 @@ void dagmcMetaData::parse_material_data() {
// implicit complement
else if (DAG->is_implicit_complement(eh)) {
if (implicit_complement_material == "") {
std::cout << "Implicit Complement assumed to be Vacuum" << std::endl;
message("Implicit Complement assumed to be Vacuum");
volume_material_property_data_eh[eh] = "mat:Vacuum";
volume_material_data_eh[eh] = vacuum_str;
} else {
Expand Down Expand Up @@ -283,12 +284,14 @@ void dagmcMetaData::parse_importance_data() {
}
importance_map[eh][pair.first] = imp;
} else {
std::cout << "Volume with ID " << volid
<< " has more than one importance " << std::endl;
std::cout << "Assigned for particle type " << pair.first << std::endl;
std::cout << "Only one importance value per volume per particle type "
"is allowed"
<< std::endl;
std::stringstream ss;
ss << "Volume with ID " << volid << " has more than one importance "
<< std::endl;
ss << "Assigned for particle type " << pair.first << std::endl;
ss << "Only one importance value per volume per particle type "
"is allowed"
<< std::endl;
message(ss.str(), -1, false);
pshriwise marked this conversation as resolved.
Show resolved Hide resolved
exit(EXIT_FAILURE);
}
}
Expand All @@ -303,11 +306,11 @@ void dagmcMetaData::parse_importance_data() {
std::string particle = *it;
moab::EntityHandle eh = DAG->entity_by_index(3, i);
if (importance_map[eh].count(particle) == 0) {
if (verbose) {
std::cout << "Warning: Volume with ID " << DAG->id_by_index(3, i);
std::cout << " does not have an importance set for particle ";
std::cout << particle << " assuming importance 1.0 " << std::endl;
}
std::stringstream ss;
ss << "Volume with ID " << DAG->id_by_index(3, i);
ss << " does not have an importance set for particle ";
ss << particle << " assuming importance 1.0 ";
message(ss.str(), 1);
// give this particle default importance
importance_map[eh][particle] = 1.0;
}
Expand Down Expand Up @@ -360,15 +363,16 @@ void dagmcMetaData::parse_boundary_data() {

boundary_assignment = boundary_assignments[eh];
if (boundary_assignment.size() != 1) {
std::cout << "More than one boundary conditions specified for " << surfid
<< std::endl;
std::cout << surfid << " has the following density assignments"
<< std::endl;
std::stringstream ss;
ss << "More than one boundary conditions specified for " << surfid
<< std::endl;
ss << surfid << " has the following density assignments" << std::endl;
for (int j = 0; j < boundary_assignment.size(); j++) {
std::cout << boundary_assignment[j] << std::endl;
ss << boundary_assignment[j] << std::endl;
}
std::cout << "Please check your boundary condition assignments " << surfid
<< std::endl;
ss << "Please check your boundary condition assignments " << surfid
<< std::endl;
err_msg(ss.str());
pshriwise marked this conversation as resolved.
Show resolved Hide resolved
exit(EXIT_FAILURE);
}
// 2d entities have been tagged with the boundary condition property
Expand Down Expand Up @@ -620,8 +624,9 @@ std::pair<std::string, std::string> dagmcMetaData::split_string(
int str_length = property_string.length() - found_delimiter;
second = property_string.substr(found_delimiter + 1, str_length);
} else {
std::cout << "Didn't find any delimiter" << std::endl;
std::cout << "Returning empty strings" << std::endl;
warning(
"Didn't find any delimiter.\n "
"Returning empty strings");
}

return {first, second};
Expand Down
53 changes: 52 additions & 1 deletion src/dagmc/dagmcmetadata.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#define SRC_DAGMC_DAGMCMETADATA_HPP_
#include <iostream>
#include <set>
#include <sstream>

#include "DagMC.hpp"

Expand Down Expand Up @@ -109,10 +110,60 @@ class dagmcMetaData {
// map of importance data
std::map<moab::EntityHandle, std::map<std::string, double>> importance_map;

// Verbosity levels
// -1 : "override" the current verbosity setting and write message
// 0 : No output
// 1 : All output
int set_verbosity(int val) {
int verbosity_max = 0;
int verbosity_min = 1;
if (val < verbosity_min || val > verbosity_max)
warning("Invalid verbosity value " + std::to_string(val) +
" will be set to nearest valid value.");
val = std::min(std::max(verbosity_min, val), verbosity_max);
return verbosity = val;
}

int get_verbosity(int val) { return verbosity; }

/** console output mechanism
* @param msg Message to be written to the console
* @param lvl Message will not be written if the verbosity level
* is higher than the class instance's current verbosity setting.
* Use of -1 ensures the message will always be written.
* @param newline Whether or not to apend a newline to the message.
*/
void message(const std::string& msg, int lvl = 1, bool newline = true) const {
// do not write message if level is higher than
// verbosity setting
if (lvl > verbosity) return;
// otherwise, display message
if (newline)
std::cout << msg << "\n";
else
std::cout << msg;
}

/** write a warning message to the console
* @param msg Message to be written to the console
* @param newline Whether or not to apend a newline to the message.
*/
void warning(const std::string& msg, bool newline = true) const {
message("WARNING: " + msg, -1, newline);
}

/** write an error message to the console
* @param msg Message to be written to the console
* @param newline Whether or not to apend a newline to the message.
*/
void err_msg(const std::string& msg, bool newline = true) const {
message("ERROR: " + msg, -1, newline);
}

// private member variables
private:
moab::DagMC* DAG; // Pointer to DAGMC instance
bool verbose; // Provide additional output while setting up and parsing
int verbosity{1}; // Provide additional output while setting up and parsing
// properties
bool require_density; // Require that all volumes have a specified density
// value
Expand Down