Feature nrtest #121
Merged
Feature nrtest #121
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… into feature-nrtest
|
@michaeltryby, Awesome work! |
|
Suggest doing a squash and merge on this one! |
|
Review of pull requests is a great way to learn and improve the quality of our work. If @samhatchett is too busy perhaps another member of the dev committee could review this. Sound good? |
|
@michaeltryby, do you think there might be value in adding more tests? |
|
@bemcdonnell Yeah I agree that more tests would be good. I asked @LRossman if he would contribute his bench marking library. Waiting to hear back from him. |
|
@michaeltryby as you know, i've been on vacation all last week. Thus the delay. Of course anyone else is welcome and encouraged to add their review as well, but I intend to look this over in the next 4-5 days. |
samhatchett
approved these changes
Oct 26, 2017
There was a problem hiding this comment.
Thanks @michaeltryby for this PR. This fulfills the functional requirements for regression testing, so we are miles ahead of where we were.
we will want the ability to specify different tolerances for different comparison params in future work, but this is a fantastic example to work from. Thanks!
|
Thanks @samhatchett! I will create an issue for the nrtest tolerance parameterization. #122 |
samhatchett
added a commit
that referenced
this pull request
Nov 8, 2017
* Initial commit EPANET testing tools. * Initial commit for epanet-nrtestsuite * SWIG wrapper for EPANET outputapi (#118) * Removed pervious version of outputapi and wrapper * SWIG wrapper for EPANET outputapi * Patching cmake build script fixed target for outputapi * Build failing on deprecated test script * Minor changes. Responding to review comments. * Feature nrtest (#121) * Configured python setup to automatically build nrtest tools. * Working on build / testing automation * Adding EPANET 2.0.12 benchmark * Updated Travis yml to run nrtest * Fixing InsecurePlatformWarning * Fixing InsecurePlatformWarning again * Fixing InsecurePlatformWarning * Fixing InsecurePlatformWarning * Fixing InsecurePlatformWarning * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update .travis.yml * Working on configuring python environment and building test tools under Travis CI. * Making gen-config.sh and run-nrtest.sh executable * Debugging .travis.yml * Debugging .travis.yml * Debugging .travis.yml again * Debugging .travis.yml again * debugging travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * Fixing bug with __strncpy_chk destlen < len * nrtesting clean up
samhatchett
added a commit
that referenced
this pull request
Jan 9, 2018
this quite sizable commit does several things, but is primarily focussed on building a toolkit that can run simultaneous simulations/analyses within a shared memory space. Versions <=2.1 use a long list of global variables that prevent multiple instantiations on linux systems without resorting to compilation tricks (like duplicate binaries or similar via static linkage). This version uses a single "Project" pointer to encapsulate the network and analysis data. There are no changes to existing algo implementations other than to accomodate dereferencing of the passed-in pointers. A more detailed list of major changes below: - mirrors all “ENxxxx” function calls with “EN_xxxx” versions (note the underscore) that take an extra first parameter: a pointer to an EN_Project struct, which contains all network, hydraulic, quality, and associated data. - tweaks some code formatting to make it more readable - removes some deprecated/commented code that was sufficiently old - fixes implicit type-cast warnings * Added ENaddnode and ENaddlink functions * More memory reallocations * Added ENInit, ENsetheadcurveindex * Added ENdeletelink and ENdeletenode * restored default behavior for float types * fixed type * Added docstrings for ENinit * cleanup change * moves global rule variables to vars.h * migrates rule structs to typedefs for better readability * char types to proper enums fixes #93 * Change some variable declarations for compatibility Changes to keep compatibility with C89 compilers: variables must be declared at the top of the functions. Remove the use of EN_LinkType in function call as it is not compatible with ENgetnodetype. * Moved declaration of idstodelete to top of function * Updated ENinit function and headers Updated header files with new functions Updated def file with new functions For ENinit changes names of parameters #98 Added enum for headloss formula * Missed these files in 1a033fc * migrates char types to enums fixes #93, supports unified link/node type enums, rather than public/private redefinitions * removing links in reverse-index order maintains proper indexing fixes #96 * style * clarifies curve getter units issue (dox) closes #95 * fixes link/node confusion in ENsetlinktype partially reverts a3bce95 * partial compilation fix * fixes dox issue * fixes allocation issues with enums - updates style in various places - introduces FlowDirection enum - use snprintf to prevent overflow * fixes enum type cast * updated mac project settings * Use of _snprintf on Windows and remove DLLEXPORT from mempool.h snprintf it not compatible on Windows so we use _snprintf mempool gave starnge compilation errors while removing DLLEXPORT worked. Not sure why these functions needed to be exposed in the DLL? * Revert "Use of _snprintf on Windows and remove DLLEXPORT from mempool.h" This reverts commit 6238f77. * use of _snprintf instead of snprintf on Windows and removed DLLEXPORT from mempool.h Had compilation errors on mempool.h. Removed DLLEXPORT so solve it. Not sure why there was a need to expose these functions? * Shift indices for Links in ENaddnode Need to shift indices for Links not just Pipes since a pump could be connected directly to a reservoir. Also set the defult base demand to zero (was 5). * Set defualts for madatory link properties in ENaddlink and small fix for ENsetheadcurveindex Relates to #102 and closes #103 * wraps globals into structs, duplicates api functions with objective versions * parse and serialize Comment field for network elements related to #47 * adds getter for head/efficiency curve in EN_getlinkvalue * adds getter for event node index … to return the index of the junction (tank) that triggered the event. * fixes edge case in parsing … where inp files without demands in [JUNCTIONS] and without any [DEMAND] categories will fail. * adds freeing function for project pointer * removes redundant string literals, fixes overrun issue in error message getter function * check for hydraulics already closed * moving error definitions to data file * deprecates ENR err message getter (unused) * updates location of errors data file also begins to expose blind structs to curves and patterns, anticipating buildout of APIs for those. * updates CLI output to reflect executable name as invoked relates to #109 * Feature nrtest (#131) * Initial commit EPANET testing tools. * Initial commit for epanet-nrtestsuite * SWIG wrapper for EPANET outputapi (#118) * Removed pervious version of outputapi and wrapper * SWIG wrapper for EPANET outputapi * Patching cmake build script fixed target for outputapi * Build failing on deprecated test script * Minor changes. Responding to review comments. * Feature nrtest (#121) * Configured python setup to automatically build nrtest tools. * Working on build / testing automation * Adding EPANET 2.0.12 benchmark * Updated Travis yml to run nrtest * Fixing InsecurePlatformWarning * Fixing InsecurePlatformWarning again * Fixing InsecurePlatformWarning * Fixing InsecurePlatformWarning * Fixing InsecurePlatformWarning * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update .travis.yml * Working on configuring python environment and building test tools under Travis CI. * Making gen-config.sh and run-nrtest.sh executable * Debugging .travis.yml * Debugging .travis.yml * Debugging .travis.yml again * Debugging .travis.yml again * debugging travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * debugging Travis setup * Fixing bug with __strncpy_chk destlen < len * nrtesting clean up * re-implements fixes from: 5eead5a 3c78856 * removes extraneous build files, moves cmake and updates travis * mirror of 9b37035 * Move some variable declarations * More variable declarations * Fix TmpDir * Allocate _defaultModel * Fix EN_addcurve funcrion * Fix for inpfile * Fix writeRuleinInp call * Set MAXMSG to 79 chars * Fix for flow direction * Refactoring testing related python packages and SWIG wrapper bug fix (#139) * Eliminated epanet-reader package. Removed numpy dependency from epanet-output. Fixed reference counting bug in SWIG wrapper. Added error checking to run_nrtest.sh. Added nrtest package to requirements file. * changing buildhome directory * Fixing bug related to preprocessor definition of PI
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses Issue #97. It wasn't pretty, but
nrtestis now running from Travis CI.Note that the tests are failing, however,
nrtestis running properly. Testing identifies differences in water quality results between the EPANET v 2.0.12 reference and the current build.