Skip to content

Commit

Permalink
Using logging functions to standardize the std::cerr output messages
Browse files Browse the repository at this point in the history
  • Loading branch information
stcui007 committed Aug 23, 2024
1 parent 088fc26 commit c8263ea
Show file tree
Hide file tree
Showing 31 changed files with 145 additions and 117 deletions.
3 changes: 2 additions & 1 deletion include/bmi/Bmi_Py_Adapter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "Bmi_Adapter.hpp"

#include "utilities/python/InterpreterUtil.hpp"
#include "utilities/logging_utils.h"

// Forward declaration to provide access to protected items in testing
class Bmi_Py_Adapter_Test;
Expand Down Expand Up @@ -685,7 +686,7 @@ namespace models {
//This message is lost and often contains valuable info. Either need to break up and catch
//other possible exceptions, wrap all these in a custom exception, or at the very least, print
//the original messge before it gets lost in this re-throw.
std::cerr<<init_exception_msg<<std::endl;
logging::error((init_exception_msg + "\n").c_str());
throw e;
}
}
Expand Down
13 changes: 7 additions & 6 deletions include/core/HY_Features.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <network.hpp>
#include <Formulation_Manager.hpp>
#include <HY_Features_Ids.hpp>
#include "utilities/logging_utils.h"

namespace hy_features {

Expand Down Expand Up @@ -173,21 +174,21 @@ namespace hy_features {
auto downstream = network.get_destination_ids(id);
if(downstream.size() > 1)
{
std::cerr << "Catchment " << id << " has more than one downstream connection." << std::endl;
std::cerr << "Downstreams are: ";
logging::error(("Catchment " + id + " has more than one downstream connection.\n").c_str());
logging::error((std::string("Downstreams are: ")).c_str());
for(const auto& id : downstream){
std::cerr <<id<<" ";
logging::formatting((id+" ").c_str());
}
std::cerr << std::endl;
logging::formatting((std::string("\n")).c_str());
assert( false );
}
else if (downstream.size() == 0)
{
std::cerr << "Catchment " << id << " has 0 downstream connections, must have 1." << std::endl;
logging::error(("Catchment " + id + " has 0 downstream connections, must have 1.\n").c_str());
assert( false );
}
}
std::cout<<"Catchment topology is dendritic."<<std::endl;
logging::critical((std::string("Catchment topology is dendritic.\n")).c_str());
}

/**
Expand Down
11 changes: 6 additions & 5 deletions include/core/HY_Features_MPI.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <network.hpp>
#include <Formulation_Manager.hpp>
#include <Partition_Parser.hpp>
#include "logging_utils.h"

namespace hy_features {

Expand Down Expand Up @@ -68,16 +69,16 @@ namespace hy_features {
for(const auto& id : catchments()) {
auto downstream = network.get_destination_ids(id);
if(downstream.size() > 1) {
std::cerr << "Catchment " << id << " has more than one downstream connection." << std::endl;
std::cerr << "Downstreams are: ";
logging::error((std::string("Catchment ") + id + " has more than one downstream connection.\n").c_str());
logging::error((std::string("Downstreams are: ")).c_str());
for(const auto& id : downstream){
std::cerr <<id<<" ";
logging::formatting((id+std::string(" ")).c_str());
}
std::cerr << std::endl;
logging::formatting((std::string("\n")).c_str());
assert( false );
}
else if (downstream.size() == 0) {
std::cerr << "Catchment " << id << " has 0 downstream connections, must have 1." << std::endl;
logging::error((std::string("Catchment ") + id + " has 0 downstream connections, must have 1.\n").c_str());
assert( false );
}
}
Expand Down
3 changes: 2 additions & 1 deletion include/forcing/CsvPerFeatureForcingProvider.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "DataProviderSelectors.hpp"
#include <exception>
#include <UnitsHelper.hpp>
#include "logging_utils.h"

/**
* @brief Forcing class providing time-series precipiation forcing data to the model.
Expand Down Expand Up @@ -165,7 +166,7 @@ class CsvPerFeatureForcingProvider : public data_access::GenericDataProvider
}
catch (const std::runtime_error& e){
#ifndef UDUNITS_QUIET
std::cerr<<"WARN: Unit conversion unsuccessful - Returning unconverted value! (\""<<e.what()<<"\")"<<std::endl;
logging::warning((std::string("WARN: Unit conversion unsuccessful - Returning unconverted value! (\"")+e.what()+"\")\n").c_str());
#endif
return value;
}
Expand Down
18 changes: 9 additions & 9 deletions include/geojson/features/CollectionFeature.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ namespace geojson {
catch (boost::bad_get &exception) {
std::string template_name = "Point";
std::string expected_name = get_geometry_type(this->geometry_collection[index]);
std::cerr << "Asked for " << template_name << ", but only " << expected_name << " is valid" << std::endl;
logging::error((std::string("Asked for ") + template_name + ", but only " + expected_name + " is valid\n").c_str());
throw;
}
}
Expand All @@ -56,7 +56,7 @@ namespace geojson {
catch (boost::bad_get &exception) {
std::string template_name = "Point";
std::string expected_name = get_geometry_type(geometry);
std::cerr << "Asked for " << template_name << ", but only " << expected_name << " is valid" << std::endl;
logging::error((std::string("Asked for ") + template_name + ", but only " + expected_name + " is valid\n").c_str());
throw;
}
}
Expand All @@ -72,7 +72,7 @@ namespace geojson {
catch (boost::bad_get &exception) {
std::string template_name = "LineString";
std::string expected_name = get_geometry_type(this->geometry_collection[index]);
std::cerr << "Asked for " << template_name << ", but only " << expected_name << " is valid" << std::endl;
logging::error((std::string("Asked for ") + template_name + ", but only " + expected_name + " is valid\n").c_str());
throw;
}
}
Expand All @@ -88,7 +88,7 @@ namespace geojson {
catch (boost::bad_get &exception) {
std::string template_name = "LineString";
std::string expected_name = get_geometry_type(geometry);
std::cerr << "Asked for " << template_name << ", but only " << expected_name << " is valid" << std::endl;
logging::error((std::string("Asked for ") + template_name + ", but only " + expected_name + " is valid\n").c_str());
throw;
}
}
Expand All @@ -104,7 +104,7 @@ namespace geojson {
catch (boost::bad_get &exception) {
std::string template_name = "Polygon";
std::string expected_name = get_geometry_type(this->geometry_collection[index]);
std::cerr << "Asked for " << template_name << ", but only " << expected_name << " is valid" << std::endl;
logging::error((std::string("Asked for ") + template_name + ", but only " + expected_name + " is valid\n").c_str());
throw;
}
}
Expand All @@ -120,7 +120,7 @@ namespace geojson {
catch (boost::bad_get &exception) {
std::string template_name = "Polygon";
std::string expected_name = get_geometry_type(geometry);
std::cerr << "Asked for " << template_name << ", but only " << expected_name << " is valid" << std::endl;
logging::error((std::string("Asked for ") + template_name + ", but only " + expected_name + " is valid\n").c_str());
throw;
}
}
Expand All @@ -136,7 +136,7 @@ namespace geojson {
catch (boost::bad_get &exception) {
std::string template_name = "MultiPoint";
std::string expected_name = get_geometry_type(this->geometry_collection[index]);
std::cerr << "Asked for " << template_name << ", but only " << expected_name << " is valid" << std::endl;
logging::error((std::string("Asked for ") + template_name + ", but only " + expected_name + " is valid\n").c_str());
throw;
}
}
Expand All @@ -160,7 +160,7 @@ namespace geojson {
catch (boost::bad_get &exception) {
std::string template_name = "MultiLineString";
std::string expected_name = get_geometry_type(this->geometry_collection[index]);
std::cerr << "Asked for " << template_name << ", but only " << expected_name << " is valid" << std::endl;
logging::error((std::string("Asked for ") + template_name + ", but only " + expected_name + " is valid\n").c_str());
throw;
}
}
Expand All @@ -184,7 +184,7 @@ namespace geojson {
catch (boost::bad_get &exception) {
std::string template_name = "MultiPolygon";
std::string expected_name = get_geometry_type(this->geometry_collection[index]);
std::cerr << "Asked for " << template_name << ", but only " << expected_name << " is valid" << std::endl;
logging::error((std::string("Asked for ") + template_name + ", but only " + expected_name + " is valid\n").c_str());
throw;
}
}
Expand Down
5 changes: 3 additions & 2 deletions include/geojson/features/FeatureBase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "JSONGeometry.hpp"
#include "JSONProperty.hpp"
#include "FeatureVisitor.hpp"
#include "logging_utils.h"

#include <memory>
#include <ostream>
Expand Down Expand Up @@ -530,7 +531,7 @@ namespace geojson {
catch (boost::bad_get &exception) {
std::string template_name = boost::typeindex::type_id<T>().pretty_name();
std::string expected_name = get_geometry_type(this->geom);
std::cerr << "Asked for " << template_name << ", but only " << expected_name << " is valid" << std::endl;
logging::error((std::string("Asked for ") + template_name + ", but only " + expected_name + " is valid\n").c_str());
throw;
}
}
Expand All @@ -547,7 +548,7 @@ namespace geojson {
catch (boost::bad_get &exception) {
std::string template_name = boost::typeindex::type_id<T>().pretty_name();
std::string expected_name = get_geometry_type(this->geometry_collection[index]);
std::cerr << "Asked for " << template_name << ", but only " << expected_name << " is valid" << std::endl;
logging::error((std::string("Asked for ") + template_name + ", but only " + expected_name + " is valid\n").c_str());
throw;
}
}
Expand Down
25 changes: 12 additions & 13 deletions include/realizations/catchment/Formulation_Manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "features/Features.hpp"
#include "Formulation_Constructors.hpp"
#include "LayerData.hpp"
#include "logging_utils.h"
#include "realizations/config/time.hpp"
#include "realizations/config/routing.hpp"
#include "realizations/config/config.hpp"
Expand Down Expand Up @@ -130,8 +131,8 @@ namespace realization {
using_routing = true;
#else
using_routing = false;
std::cerr<<"WARNING: Formulation Manager found routing configuration"
<<", but routing support isn't enabled. No routing will occur."<<std::endl;
logging::warning((std::string("WARNING: Formulation Manager found routing configuration")
+", but routing support isn't enabled. No routing will occur.\n").c_str());
#endif //NGEN_WITH_ROUTING
}

Expand All @@ -146,9 +147,9 @@ namespace realization {
if( catchment_index == -1 )
{
#ifndef NGEN_QUIET
std::cerr<<"WARNING Formulation_Manager::read: Cannot create formulation for catchment "
<<catchment_config.first
<<" that isn't identified in the hydrofabric or requested subset"<<std::endl;
logging::warning((std::string("WARNING Formulation_Manager::read: Cannot create formulation for catchment ")
+catchment_config.first
+" that isn't identified in the hydrofabric or requested subset\n").c_str());
#endif
continue;
}
Expand Down Expand Up @@ -577,15 +578,14 @@ namespace realization {
case geojson::PropertyType::List:
case geojson::PropertyType::Object:
default:
std::cerr << "WARNING: property type " << static_cast<int>(catchment_attribute.get_type()) << " not allowed as model parameter. "
<< "Must be one of: Natural (int), Real (double), Boolean, or String" << '\n';
logging::error((std::string("WARNING: property type ") + std::to_string(static_cast<int>(catchment_attribute.get_type())) + " not allowed as model parameter. " + "Must be one of: Natural (int), Real (double), Boolean, or String\n").c_str());
break;
}
} else {
std::cerr << "WARNING Failed to parse external parameter: catchment `"
<< catchment_feature->get_id()
<< "` does not contain the property `"
<< param_name << "`\n";
logging::error((std::string("WARNING Failed to parse external parameter: catchment `")
+ catchment_feature->get_id()
+ "` does not contain the property `"
+ param_name + "`\n").c_str());
}
}

Expand Down Expand Up @@ -646,8 +646,7 @@ namespace realization {
default:
// TODO: Should list/object values be passed to model parameters?
// Typically, feature properties *should* be scalars.
std::cerr << "WARNING: property type " << static_cast<int>(catchment_attribute.get_type()) << " not allowed as model parameter. "
<< "Must be one of: Natural (int), Real (double), Boolean, or String" << '\n';
logging::error((std::string("WARNING: property type ") + std::to_string(static_cast<int>(catchment_attribute.get_type())) + " not allowed as model parameter. " + "Must be one of: Natural (int), Real (double), Boolean, or String\n").c_str());
break;
}
}
Expand Down
3 changes: 1 addition & 2 deletions include/realizations/config/formulation.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,7 @@ namespace realization{
case geojson::PropertyType::Object:
// TODO: Should list/object values be passed to model parameters?
// Typically, feature properties *should* be scalars.
std::cerr << "WARNING: property type " << static_cast<int>(catchment_attribute.get_type()) << " not allowed as model parameter. "
<< "Must be one of: Natural (int), Real (double), Boolean, or String" << '\n';
logging::error((std::string("WARNING: property type ") + std::to_string(static_cast<int>(catchment_attribute.get_type())) + " not allowed as model parameter. " + "Must be one of: Natural (int), Real (double), Boolean, or String\n").c_str());
break;
default:
attr.at(param.first) = geojson::JSONProperty(param.first, catchment_attribute);
Expand Down
6 changes: 6 additions & 0 deletions include/utilities/logging_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ namespace logging {
* @param msg The variable carries the humanly readable critical message.
*/
void critical(const char* msg);

/**
* Used for pretty printting when std::cerr has multi-line output
* @param msg The variable carries the humanly readable message.
*/
void formatting(const char* msg);
}

#ifdef __cplusplus
Expand Down
7 changes: 4 additions & 3 deletions include/utilities/parallel_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define NGEN_MPI_PROTOCOL_TAG 101
#endif

#include "utilities/logging_utils.h"
#include <cstring>
#include <mpi.h>
#include <string>
Expand Down Expand Up @@ -479,7 +480,7 @@ namespace parallel {
#if !NGEN_WITH_PYTHON
// We can't be good to proceed with this, because Python is not active
isGood = false;
std::cerr << "Driver is unable to perform required hydrofabric subdividing when Python integration is not active." << std::endl;
logging::critical((std::string("Driver is unable to perform required hydrofabric subdividing when Python integration is not active.\n")).c_str());


// Sync with the rest of the ranks and bail if any aren't ready to proceed for any reason
Expand All @@ -496,7 +497,7 @@ namespace parallel {
partitionConfigFile);
}
catch (const std::exception &e) {
std::cerr << e.what() << std::endl;
logging::error((e.what() + std::string("\n")).c_str());
// Set not good if the subdivider object couldn't be instantiated
isGood = false;
}
Expand All @@ -512,7 +513,7 @@ namespace parallel {
isGood = subdivider->execSubdivision();
}
catch (const std::exception &e) {
std::cerr << e.what() << std::endl;
logging::error((e.what() + std::string("\n")).c_str());
// Set not good if the subdivider object couldn't be instantiated
isGood = false;
}
Expand Down
5 changes: 3 additions & 2 deletions include/utilities/python/HydrofabricSubsetter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <pybind11/embed.h>
#include "InterpreterUtil.hpp"
#include "utilities/logging_utils.h"

namespace py = pybind11;

Expand Down Expand Up @@ -96,7 +97,7 @@ namespace utils {
result = bool_result;
}
catch (const std::exception &e) {
std::cerr << "Failed to subdivide hydrofabric: " << e.what() << std::endl;
logging::error((std::string("Failed to subdivide hydrofabric: ") + e.what()).c_str());
result = false;
}
return result;
Expand All @@ -109,7 +110,7 @@ namespace utils {
result = bool_result;
}
catch (const std::exception &e) {
std::cerr << "Failed to subdivide hydrofabric for index " << index << ": " << e.what() << std::endl;
logging::error((std::string("Failed to subdivide hydrofabric for index ") + std::to_string(index) + ": " + e.what() + "\n").c_str());
result = false;
}
return result;
Expand Down
Loading

0 comments on commit c8263ea

Please sign in to comment.