Build MCNP Shared Library #33
Conversation
…c code. Started to include case-by-case type reduction for more friendly CAD types if they are geometrically equivalent.
…th delta no matter what.
… code from CMakeLists.txt.
| @@ -54,6 +54,7 @@ static int makeint( const std::string& token ){ | |||
| char* end; | |||
| int ret = strtol(str, &end, 10); | |||
| if( end != str+token.length() ){ | |||
| record << "Warning: string [" << token << "] did not convert to int as expected." << std::endl; | |||
| std::cerr << "Warning: string [" << token << "] did not convert to int as expected." << std::endl; | |||
There was a problem hiding this comment.
The ghost of hackers past moans:
These duplicated printouts make me sad. Consider defining
// Note: this macro will evaluate its argument expression twice!
#define NOISY_WARNING(OUTPUT_EXPR) do{ \
std::cerr << "Warning: " << OUTPUT_EXPR << std::endl; \
record << "Warning: " << OUTPUT_EXPR << std::endl; \
} while (0)Then these bug-prone output copypastas can become simply
NOISY_WARNING( "string [" << token << "] did not convert to int as expected" );There was a problem hiding this comment.
It was actually decided to remove the cerr outputs entirely; those changes will be in the next PR, but I could make another small commit to this one removing them as well.
There was a problem hiding this comment.
Don't bother -- if you're planning to clean it up in in a future PR that's fine.
(Output and logging in c++ is always a colorful bikeshed. In the past I've written custom ostream subclasses that can optionally direct to zero, one, or multiple outputs (stdout, file, etc.), but it's a hassle to maintain. Lavtely I've found it most comfortable to create macros that accept ostream syntax. This provides a nice way to centrally control verbosity, write to multiple outputs, compile out DEBUG statements, etc.)
MCNPInput.hpp
Outdated
| @@ -3,12 +3,14 @@ | |||
|
|
|||
| #include <vector> | |||
| #include <map> | |||
| #include <iosfwd> | |||
| //#include <iosfwd> | |||
There was a problem hiding this comment.
Can these be removed entirely rather than being commented?
|
Can we move the CLI files into a subdirectory - perhaps called |
CMakeLists.txt
Outdated
| endif() | ||
|
|
||
| # Get the packages needed to use the SDK | ||
| find_package(Cubit REQUIRED CONFIG) |
There was a problem hiding this comment.
Why is this required? Do we perhaps need a more generic version of this to find some form of iGeom?
There was a problem hiding this comment.
There are a number of header files needed by iGeom that are found in Trelis. iGeom.h needs to include RefGroup.hpp for much of the CATag parts, which in turn includes more files.
mcnp2cad also needs GeometryModifyTool.hpp (which include I had erroneously put into iGeom.h) in order to take advantage of the PRINT_INFO macro, without which we would have to find another way to print to the Trelis command line, if we want to do that.
There was a problem hiding this comment.
but this will now be a stand-alone library that needs to know where to find iGeom and hopefully upon doing so, is told where to find the other includes
There was a problem hiding this comment.
per our discussion: change Cubit to CGM and then use CGM_*_DIR throughout.
CMakeLists.txt
Outdated
|
|
||
| # These directories need to be defined by the user. | ||
| include_directories(${IGEOM_INCLUDE_DIR}) | ||
| include_directories(${MOAB_INCLUDE_DIR}) |
There was a problem hiding this comment.
Why do we need MOAB includes? I thought this depend only iGeom?
There was a problem hiding this comment.
I had been linking to the MOAB includes for iBase.h, since iGeom uses iBase_EntityHandles to keep track of everything. I can actually just take iBase.h from MOAB; it's only about 500 lines. I was trying to avoid copying files as much as possible, but it's probably worth it to avoid having to have MOAB installed as well if one isn't using the plugin's iGeom.
There was a problem hiding this comment.
Doesn't iGeom have it's own iBase?
There was a problem hiding this comment.
|
We probably need to revise the README to reflect these changes. |
|
Replaced by #53 |
This pull request is focused on the functionality of the code to turn MCNP2CAD into a library able to be called by a Trelis-Plugin. There is another pull request planned for the future with fixes to formatting and commenting issues.