From 8140ff492c5a25b6908f4f1178e2677c436cb592 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Wed, 10 Aug 2022 14:23:26 -0500 Subject: [PATCH 01/32] added some commenting and the extern remove_tally_from_tallies function --- include/openmc/tallies/tally.h | 4 ++-- src/tallies/tally.cpp | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/include/openmc/tallies/tally.h b/include/openmc/tallies/tally.h index 73618a3da..46284dea2 100644 --- a/include/openmc/tallies/tally.h +++ b/include/openmc/tallies/tally.h @@ -47,9 +47,9 @@ public: void set_nuclides(const vector& nuclides); - const vector& filters() const { return filters_; } + const vector& filters() const { return filters_; } // returns vector of inidices corresponding to the tally this is called on - int32_t filters(int i) const { return filters_[i]; } + int32_t filters(int i) const { return filters_[i]; } // i corresponds to the index of the filter for this tally void set_filters(gsl::span filters); diff --git a/src/tallies/tally.cpp b/src/tallies/tally.cpp index c0fedd973..82b526b4e 100644 --- a/src/tallies/tally.cpp +++ b/src/tallies/tally.cpp @@ -46,6 +46,8 @@ namespace openmc { //============================================================================== namespace model { +// map: key (tally ID) to value (tally index) +// index coresponds to the tally ID's posiition in the tallies vector std::unordered_map tally_map; vector> tallies; vector active_tallies; @@ -1271,4 +1273,13 @@ extern "C" size_t tallies_size() return model::tallies.size(); } +// given a tally ID, remove it from the tallies vector +extern "C" void remove_tally_from_tallies(int32_t id) +{ + // query map for index corersponding to the given id + int index = model::tally_map[id]; + // delete the tally + model::tallies.erase(index); +} + } // namespace openmc From 9d146a3e0f657a7d02c231a13cdff65b025b0ded Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Wed, 10 Aug 2022 15:44:00 -0500 Subject: [PATCH 02/32] added openmc_remove_tally_from_tallies to C++ API in tally.py --- openmc/lib/tally.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openmc/lib/tally.py b/openmc/lib/tally.py index df28cb5dc..d839d6ddc 100644 --- a/openmc/lib/tally.py +++ b/openmc/lib/tally.py @@ -87,6 +87,9 @@ _dll.openmc_tally_set_type.errcheck = _error_handler _dll.openmc_tally_set_writable.argtypes = [c_int32, c_bool] _dll.openmc_tally_set_writable.restype = c_int _dll.openmc_tally_set_writable.errcheck = _error_handler +_dll.openmc_remove_tally_from_tallies.argtypes = [c_int32] +_dll.openmc_remove_tally_from_tallies.restype = None +_dll.openmc_remove_tally_from_tallies.errcheck = _error_handler _dll.tallies_size.restype = c_size_t From e3e2c301afdb7838d14521c4eefc113965a24acd Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Wed, 10 Aug 2022 15:59:22 -0500 Subject: [PATCH 03/32] forgot to prefix function with openmc. all other extern functions do this,so I changed it to be parallel --- src/tallies/tally.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tallies/tally.cpp b/src/tallies/tally.cpp index 82b526b4e..4384d62c7 100644 --- a/src/tallies/tally.cpp +++ b/src/tallies/tally.cpp @@ -1274,7 +1274,7 @@ extern "C" size_t tallies_size() } // given a tally ID, remove it from the tallies vector -extern "C" void remove_tally_from_tallies(int32_t id) +extern "C" void openmc_remove_tally_from_tallies(int32_t id) { // query map for index corersponding to the given id int index = model::tally_map[id]; From 6d9d665e53569110894f1d7896304cbed4d8a90e Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Wed, 10 Aug 2022 16:29:33 -0500 Subject: [PATCH 04/32] added new action to delete a tally as well as an assertion that the length is zero after deletion --- tests/unit_tests/test_tallies.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/unit_tests/test_tallies.py b/tests/unit_tests/test_tallies.py index bfff741a9..77aef78b4 100644 --- a/tests/unit_tests/test_tallies.py +++ b/tests/unit_tests/test_tallies.py @@ -39,3 +39,6 @@ def test_xml_roundtrip(run_in_tmpdir): assert new_tally.triggers[0].trigger_type == tally.triggers[0].trigger_type assert new_tally.triggers[0].threshold == tally.triggers[0].threshold assert new_tally.triggers[0].scores == tally.triggers[0].scores + # delete tally and check that new_tallies now has length zero + new_tally.openmc_remove_tally_from_tallies(tally.id) + assert len(new_tallies) == 0 \ No newline at end of file From 8887023b22239404c1a5d1e47883ea1231c1db9c Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Wed, 10 Aug 2022 16:45:53 -0500 Subject: [PATCH 05/32] accidental typo --- src/tallies/tally.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tallies/tally.cpp b/src/tallies/tally.cpp index 4384d62c7..f5edf75b2 100644 --- a/src/tallies/tally.cpp +++ b/src/tallies/tally.cpp @@ -47,7 +47,7 @@ namespace openmc { namespace model { // map: key (tally ID) to value (tally index) -// index coresponds to the tally ID's posiition in the tallies vector +// index coresponds to the tally ID's position in the tallies vector std::unordered_map tally_map; vector> tallies; vector active_tallies; From 99027fdd1531d634ca53af83e9e006df331ad9f5 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Wed, 10 Aug 2022 17:35:30 -0500 Subject: [PATCH 06/32] erase needs an iterator, so feed it model::tallies.begin()+index --- src/tallies/tally.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tallies/tally.cpp b/src/tallies/tally.cpp index f5edf75b2..0df00bbe7 100644 --- a/src/tallies/tally.cpp +++ b/src/tallies/tally.cpp @@ -1277,9 +1277,9 @@ extern "C" size_t tallies_size() extern "C" void openmc_remove_tally_from_tallies(int32_t id) { // query map for index corersponding to the given id - int index = model::tally_map[id]; - // delete the tally - model::tallies.erase(index); + int32_t index = model::tally_map[id]; + // delete the tally via iterator pointing to correct position + model::tallies.erase(model::tallies.begin() + index); } } // namespace openmc From 3ff11280a11f71385360fd1c04ad5bcaec0ba04b Mon Sep 17 00:00:00 2001 From: Lewis Gross <43077972+lewisgross1296@users.noreply.github.com> Date: Thu, 11 Aug 2022 13:26:12 -0500 Subject: [PATCH 07/32] documentation fix tally.h Co-authored-by: Patrick Shriwise --- include/openmc/tallies/tally.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/openmc/tallies/tally.h b/include/openmc/tallies/tally.h index 46284dea2..bc7ecaa92 100644 --- a/include/openmc/tallies/tally.h +++ b/include/openmc/tallies/tally.h @@ -47,7 +47,8 @@ public: void set_nuclides(const vector& nuclides); - const vector& filters() const { return filters_; } // returns vector of inidices corresponding to the tally this is called on + //! returns vector of indices corresponding to the tally this is called on + const vector& filters() const { return filters_; } int32_t filters(int i) const { return filters_[i]; } // i corresponds to the index of the filter for this tally From 6ef62fcaf100895defa6cc72904beae76b929d4c Mon Sep 17 00:00:00 2001 From: Lewis Gross <43077972+lewisgross1296@users.noreply.github.com> Date: Thu, 11 Aug 2022 13:26:29 -0500 Subject: [PATCH 08/32] documentation fix tally.h Co-authored-by: Patrick Shriwise --- include/openmc/tallies/tally.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/openmc/tallies/tally.h b/include/openmc/tallies/tally.h index bc7ecaa92..3cead91dc 100644 --- a/include/openmc/tallies/tally.h +++ b/include/openmc/tallies/tally.h @@ -50,7 +50,8 @@ public: //! returns vector of indices corresponding to the tally this is called on const vector& filters() const { return filters_; } - int32_t filters(int i) const { return filters_[i]; } // i corresponds to the index of the filter for this tally + //! \brief Returns the tally filter at index i + int32_t filters(int i) const { return filters_[i]; } void set_filters(gsl::span filters); From 373049601802b7567bedb719aaeef687aa6aee96 Mon Sep 17 00:00:00 2001 From: Lewis Gross <43077972+lewisgross1296@users.noreply.github.com> Date: Thu, 11 Aug 2022 13:26:50 -0500 Subject: [PATCH 09/32] documentation fix tally.cpp Co-authored-by: Patrick Shriwise --- src/tallies/tally.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tallies/tally.cpp b/src/tallies/tally.cpp index 0df00bbe7..72c4fa4fe 100644 --- a/src/tallies/tally.cpp +++ b/src/tallies/tally.cpp @@ -46,8 +46,7 @@ namespace openmc { //============================================================================== namespace model { -// map: key (tally ID) to value (tally index) -// index coresponds to the tally ID's position in the tallies vector +//! a mapping of tally ID to index in the tallies vector std::unordered_map tally_map; vector> tallies; vector active_tallies; From 30b1ae92cb0056cbf6e9084aa3c6f41b30c9898e Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Thu, 11 Aug 2022 13:45:28 -0500 Subject: [PATCH 10/32] changed name and return type. added signature to capi.h. TODO error handling in tally.cpp --- include/openmc/capi.h | 1 + src/tallies/tally.cpp | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/include/openmc/capi.h b/include/openmc/capi.h index 69d3ff1f6..e04d4898e 100644 --- a/include/openmc/capi.h +++ b/include/openmc/capi.h @@ -118,6 +118,7 @@ int openmc_regular_mesh_get_params( int openmc_regular_mesh_set_dimension(int32_t index, int n, const int* dims); int openmc_regular_mesh_set_params(int32_t index, int n, const double* ll, const double* ur, const double* width); +int openmc_remove_tally(int32_t id); int openmc_reset(); int openmc_reset_timers(); int openmc_run(); diff --git a/src/tallies/tally.cpp b/src/tallies/tally.cpp index 72c4fa4fe..8d54570ef 100644 --- a/src/tallies/tally.cpp +++ b/src/tallies/tally.cpp @@ -1273,12 +1273,13 @@ extern "C" size_t tallies_size() } // given a tally ID, remove it from the tallies vector -extern "C" void openmc_remove_tally_from_tallies(int32_t id) +extern "C" int openmc_remove_tally(int32_t id) { // query map for index corersponding to the given id int32_t index = model::tally_map[id]; // delete the tally via iterator pointing to correct position model::tallies.erase(model::tallies.begin() + index); + return 0; } } // namespace openmc From 4faaa4062b76069a03956137927b45b1073f9d1e Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Thu, 11 Aug 2022 14:03:21 -0500 Subject: [PATCH 11/32] moved test out of python API into C++ API. modeled after test that adds a new tally. TODO add a test that tries to delete an invalid ID --- tests/unit_tests/test_lib.py | 14 ++++++++++++++ tests/unit_tests/test_tallies.py | 5 +---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/unit_tests/test_lib.py b/tests/unit_tests/test_lib.py index b57242983..225415e56 100644 --- a/tests/unit_tests/test_lib.py +++ b/tests/unit_tests/test_lib.py @@ -339,6 +339,20 @@ def test_new_tally(lib_init): new_tally_with_id.scores = ['flux'] assert len(openmc.lib.tallies) == 5 +def test_add_and_delete_tally(lib_init): + with pytest.raises(exc.AllocationError): + openmc.lib.Material(1) + new_tally = openmc.lib.Tally() + new_tally.scores = ['flux'] + new_tally_with_id = openmc.lib.Tally(10) + new_tally_with_id.scores = ['flux'] + assert len(openmc.lib.tallies) == 5 + # delete tally and check length is one less than before + openmc_remove_tally(10) + assert len(openmc.lib.tallies) == 4 + +# def test_delete_wrong_id(): + def test_tally_activate(lib_simulation_init): t = openmc.lib.tallies[1] diff --git a/tests/unit_tests/test_tallies.py b/tests/unit_tests/test_tallies.py index 77aef78b4..14319a0a2 100644 --- a/tests/unit_tests/test_tallies.py +++ b/tests/unit_tests/test_tallies.py @@ -38,7 +38,4 @@ def test_xml_roundtrip(run_in_tmpdir): assert len(new_tally.triggers) == 1 assert new_tally.triggers[0].trigger_type == tally.triggers[0].trigger_type assert new_tally.triggers[0].threshold == tally.triggers[0].threshold - assert new_tally.triggers[0].scores == tally.triggers[0].scores - # delete tally and check that new_tallies now has length zero - new_tally.openmc_remove_tally_from_tallies(tally.id) - assert len(new_tallies) == 0 \ No newline at end of file + assert new_tally.triggers[0].scores == tally.triggers[0].scores \ No newline at end of file From fd2ae7811a30bcc293f488514168dad4ead42ec2 Mon Sep 17 00:00:00 2001 From: Lewis Gross <43077972+lewisgross1296@users.noreply.github.com> Date: Thu, 11 Aug 2022 14:05:23 -0500 Subject: [PATCH 12/32] use safer .at(id) Co-authored-by: Patrick Shriwise --- src/tallies/tally.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tallies/tally.cpp b/src/tallies/tally.cpp index 8d54570ef..e3baaea91 100644 --- a/src/tallies/tally.cpp +++ b/src/tallies/tally.cpp @@ -1276,7 +1276,7 @@ extern "C" size_t tallies_size() extern "C" int openmc_remove_tally(int32_t id) { // query map for index corersponding to the given id - int32_t index = model::tally_map[id]; + int32_t index = model::tally_map.at(id); // delete the tally via iterator pointing to correct position model::tallies.erase(model::tallies.begin() + index); return 0; From bce2c2384a63aba00ab4b999d6c38927ffe2cd43 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Thu, 11 Aug 2022 14:14:09 -0500 Subject: [PATCH 13/32] added error checking to ensure ID is in map before erasing. also removes the key/value pair from the map after deleting the tally --- src/tallies/tally.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/tallies/tally.cpp b/src/tallies/tally.cpp index e3baaea91..1c1b978a1 100644 --- a/src/tallies/tally.cpp +++ b/src/tallies/tally.cpp @@ -1275,11 +1275,17 @@ extern "C" size_t tallies_size() // given a tally ID, remove it from the tallies vector extern "C" int openmc_remove_tally(int32_t id) { + // check that id is in the map + if (!model::tally_map.contains(id)) { + return OPENMC_INVALID_ID; + } // query map for index corersponding to the given id int32_t index = model::tally_map.at(id); // delete the tally via iterator pointing to correct position model::tallies.erase(model::tallies.begin() + index); - return 0; + + // after erasing tally, remove id from map + model::tally_map.remove(id) return 0; } } // namespace openmc From 03806675c9701ba9eeb7eaad65445f1a114dc2cb Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Thu, 11 Aug 2022 14:32:58 -0500 Subject: [PATCH 14/32] forgot semicolon --- src/tallies/tally.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tallies/tally.cpp b/src/tallies/tally.cpp index 1c1b978a1..1613bd318 100644 --- a/src/tallies/tally.cpp +++ b/src/tallies/tally.cpp @@ -1285,7 +1285,8 @@ extern "C" int openmc_remove_tally(int32_t id) model::tallies.erase(model::tallies.begin() + index); // after erasing tally, remove id from map - model::tally_map.remove(id) return 0; + model::tally_map.erase(id); + return 0; } } // namespace openmc From 21f7cda6bc7039975d5c600e0c965850a1c4b181 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Thu, 11 Aug 2022 14:50:57 -0500 Subject: [PATCH 15/32] changed return type to match signature change --- openmc/lib/tally.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmc/lib/tally.py b/openmc/lib/tally.py index d839d6ddc..07989d558 100644 --- a/openmc/lib/tally.py +++ b/openmc/lib/tally.py @@ -88,7 +88,7 @@ _dll.openmc_tally_set_writable.argtypes = [c_int32, c_bool] _dll.openmc_tally_set_writable.restype = c_int _dll.openmc_tally_set_writable.errcheck = _error_handler _dll.openmc_remove_tally_from_tallies.argtypes = [c_int32] -_dll.openmc_remove_tally_from_tallies.restype = None +_dll.openmc_remove_tally_from_tallies.restype = c_int _dll.openmc_remove_tally_from_tallies.errcheck = _error_handler _dll.tallies_size.restype = c_size_t From f8d87b1bb30a667d8b2799c18f1e3ac5f4ffae07 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Thu, 11 Aug 2022 20:29:02 -0500 Subject: [PATCH 16/32] typo in return type --- src/tallies/tally.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tallies/tally.cpp b/src/tallies/tally.cpp index 1613bd318..3e255302e 100644 --- a/src/tallies/tally.cpp +++ b/src/tallies/tally.cpp @@ -1277,7 +1277,7 @@ extern "C" int openmc_remove_tally(int32_t id) { // check that id is in the map if (!model::tally_map.contains(id)) { - return OPENMC_INVALID_ID; + return OPENMC_E_INVALID_ID; } // query map for index corersponding to the given id int32_t index = model::tally_map.at(id); From 84359e334040b163cd692fe895f616cba5cedd86 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Thu, 11 Aug 2022 20:57:23 -0500 Subject: [PATCH 17/32] apparently count does not exist in c++11 which the github test system is using, so I will use the older trick of checking whether count equals 0 or 1 to determine if the id exists as a key --- src/tallies/tally.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tallies/tally.cpp b/src/tallies/tally.cpp index 3e255302e..73d7ae14c 100644 --- a/src/tallies/tally.cpp +++ b/src/tallies/tally.cpp @@ -1276,7 +1276,7 @@ extern "C" size_t tallies_size() extern "C" int openmc_remove_tally(int32_t id) { // check that id is in the map - if (!model::tally_map.contains(id)) { + if (model::tally_map.count(id)!=1) { return OPENMC_E_INVALID_ID; } // query map for index corersponding to the given id From 74e093ae456349798b8b2e7f666aba593a23a811 Mon Sep 17 00:00:00 2001 From: Lewis Gross <43077972+lewisgross1296@users.noreply.github.com> Date: Fri, 12 Aug 2022 10:55:21 -0500 Subject: [PATCH 18/32] renamed function and forgot to update these commands Co-authored-by: Paul Romano --- openmc/lib/tally.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openmc/lib/tally.py b/openmc/lib/tally.py index 07989d558..534e6ebaa 100644 --- a/openmc/lib/tally.py +++ b/openmc/lib/tally.py @@ -87,9 +87,9 @@ _dll.openmc_tally_set_type.errcheck = _error_handler _dll.openmc_tally_set_writable.argtypes = [c_int32, c_bool] _dll.openmc_tally_set_writable.restype = c_int _dll.openmc_tally_set_writable.errcheck = _error_handler -_dll.openmc_remove_tally_from_tallies.argtypes = [c_int32] -_dll.openmc_remove_tally_from_tallies.restype = c_int -_dll.openmc_remove_tally_from_tallies.errcheck = _error_handler +_dll.openmc_remove_tally.argtypes = [c_int32] +_dll.openmc_remove_tally.restype = c_int +_dll.openmc_remove_tally.errcheck = _error_handler _dll.tallies_size.restype = c_size_t From 7d429ab45e0092d433c3b3f80da4ec351b9699de Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Fri, 12 Aug 2022 12:58:37 -0500 Subject: [PATCH 19/32] pass index instead of ID to remove tally from tallies and tally_map --- src/tallies/tally.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/tallies/tally.cpp b/src/tallies/tally.cpp index 73d7ae14c..8f0f254e0 100644 --- a/src/tallies/tally.cpp +++ b/src/tallies/tally.cpp @@ -1273,19 +1273,18 @@ extern "C" size_t tallies_size() } // given a tally ID, remove it from the tallies vector -extern "C" int openmc_remove_tally(int32_t id) +extern "C" int openmc_remove_tally(int32_t index) { // check that id is in the map - if (model::tally_map.count(id)!=1) { - return OPENMC_E_INVALID_ID; + if (index <= 0 || index > model::tallies.size()) { + return OPENMC_E_OUT_OF_BOUNDS; } - // query map for index corersponding to the given id - int32_t index = model::tally_map.at(id); + // grab tally so it's ID can be obtained to remove the (ID,index) pair from tally_map + auto& tally = model::tallies[index]; + model::tally_map.erase(tally->id_); // delete the tally via iterator pointing to correct position model::tallies.erase(model::tallies.begin() + index); - // after erasing tally, remove id from map - model::tally_map.erase(id); return 0; } From 88c4f88abd13e929eab48325a8f13b97c86ed7a5 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Fri, 12 Aug 2022 13:02:04 -0500 Subject: [PATCH 20/32] accidentally had <= when it should be strictly < --- src/tallies/tally.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tallies/tally.cpp b/src/tallies/tally.cpp index 8f0f254e0..f45fae994 100644 --- a/src/tallies/tally.cpp +++ b/src/tallies/tally.cpp @@ -1276,7 +1276,7 @@ extern "C" size_t tallies_size() extern "C" int openmc_remove_tally(int32_t index) { // check that id is in the map - if (index <= 0 || index > model::tallies.size()) { + if (index < 0 || index > model::tallies.size()) { return OPENMC_E_OUT_OF_BOUNDS; } // grab tally so it's ID can be obtained to remove the (ID,index) pair from tally_map From 9d82e412da19b83cda05106dceceb0df8fad8c32 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Fri, 12 Aug 2022 13:24:29 -0500 Subject: [PATCH 21/32] added test to delete known tally. added test to throw an error if you remove tally that was not added --- tests/unit_tests/test_lib.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/tests/unit_tests/test_lib.py b/tests/unit_tests/test_lib.py index 225415e56..96d54c416 100644 --- a/tests/unit_tests/test_lib.py +++ b/tests/unit_tests/test_lib.py @@ -339,19 +339,16 @@ def test_new_tally(lib_init): new_tally_with_id.scores = ['flux'] assert len(openmc.lib.tallies) == 5 -def test_add_and_delete_tally(lib_init): - with pytest.raises(exc.AllocationError): - openmc.lib.Material(1) - new_tally = openmc.lib.Tally() - new_tally.scores = ['flux'] - new_tally_with_id = openmc.lib.Tally(10) - new_tally_with_id.scores = ['flux'] - assert len(openmc.lib.tallies) == 5 - # delete tally and check length is one less than before - openmc_remove_tally(10) +def test_delete_tally(lib_init): + # delete tally 10 which was added in the above test + # check length is one less than before + openmc.lib.openmc_remove_tally(openmc.lib.get_tally_index(10)) assert len(openmc.lib.tallies) == 4 -# def test_delete_wrong_id(): +def test_delete_invalid_id(lib_init): + # attempt to delete a tally that is guaranteed not to have a valid index + with pytest.raises(exc.InvalidIDError): + openmc.lib.openmc_remove_tally(np.max(id)+1) def test_tally_activate(lib_simulation_init): From d1775181e905054677d904acb9f4a652d0195325 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Fri, 12 Aug 2022 15:49:08 -0500 Subject: [PATCH 22/32] added deletion operator to TallyMapping class. used in tests and changed delete invalid id to access invalid id --- openmc/lib/tally.py | 22 +++++++++++++++------- tests/unit_tests/test_lib.py | 13 ++++++++----- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/openmc/lib/tally.py b/openmc/lib/tally.py index 534e6ebaa..c1c8ab094 100644 --- a/openmc/lib/tally.py +++ b/openmc/lib/tally.py @@ -390,13 +390,7 @@ class Tally(_FortranObjectWithID): class _TallyMapping(Mapping): def __getitem__(self, key): - index = c_int32() - try: - _dll.openmc_get_tally_index(key, index) - except (AllocationError, InvalidIDError) as e: - # __contains__ expects a KeyError to work correctly - raise KeyError(str(e)) - return Tally(index=index.value) + return Tally(index=self._get_tally_index(key)) def __iter__(self): for i in range(len(self)): @@ -408,4 +402,18 @@ class _TallyMapping(Mapping): def __repr__(self): return repr(dict(self)) + def __delitem__(self,key): + """Delete a tally from tally vector and remove the ID,index pair from tally""" + _dll.openmc_remove_tally(self._get_tally_index(key)) + + def _get_tally_index(self,key): + """Given the ID, return the index""" + index = c_int32() + try: + _dll.openmc_get_tally_index(key, index) + except (AllocationError, InvalidIDError) as e: + # __contains__ expects a KeyError to work correctly + raise KeyError(str(e)) + return index.value + tallies = _TallyMapping() diff --git a/tests/unit_tests/test_lib.py b/tests/unit_tests/test_lib.py index 96d54c416..5063646f7 100644 --- a/tests/unit_tests/test_lib.py +++ b/tests/unit_tests/test_lib.py @@ -339,16 +339,19 @@ def test_new_tally(lib_init): new_tally_with_id.scores = ['flux'] assert len(openmc.lib.tallies) == 5 + def test_delete_tally(lib_init): # delete tally 10 which was added in the above test # check length is one less than before - openmc.lib.openmc_remove_tally(openmc.lib.get_tally_index(10)) + del openmc.lib.tallies[10] assert len(openmc.lib.tallies) == 4 -def test_delete_invalid_id(lib_init): - # attempt to delete a tally that is guaranteed not to have a valid index - with pytest.raises(exc.InvalidIDError): - openmc.lib.openmc_remove_tally(np.max(id)+1) + +def test_invalid_tally_id(lib_init): + # attempt to access a tally that is guaranteed not to have a valid index + max_id = max(openmc.lib.tallies.keys()) + with pytest.raises(KeyError): + openmc.lib.tallies[max_id+1] def test_tally_activate(lib_simulation_init): From 70598087cd8d735c7fd8d122893dcd74b10a7d43 Mon Sep 17 00:00:00 2001 From: Lewis Gross <43077972+lewisgross1296@users.noreply.github.com> Date: Mon, 15 Aug 2022 13:33:49 -0500 Subject: [PATCH 23/32] forgot to change name of arrgument from id to index in header Co-authored-by: Paul Romano --- include/openmc/capi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/openmc/capi.h b/include/openmc/capi.h index e04d4898e..29e900965 100644 --- a/include/openmc/capi.h +++ b/include/openmc/capi.h @@ -118,7 +118,7 @@ int openmc_regular_mesh_get_params( int openmc_regular_mesh_set_dimension(int32_t index, int n, const int* dims); int openmc_regular_mesh_set_params(int32_t index, int n, const double* ll, const double* ur, const double* width); -int openmc_remove_tally(int32_t id); +int openmc_remove_tally(int32_t index); int openmc_reset(); int openmc_reset_timers(); int openmc_run(); From c772972808674b94f602af5fe854adcf4da3d3e3 Mon Sep 17 00:00:00 2001 From: Lewis Gross <43077972+lewisgross1296@users.noreply.github.com> Date: Mon, 15 Aug 2022 13:34:19 -0500 Subject: [PATCH 24/32] add space for compliant formatting Co-authored-by: Paul Romano --- openmc/lib/tally.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmc/lib/tally.py b/openmc/lib/tally.py index c1c8ab094..0be2f9ed5 100644 --- a/openmc/lib/tally.py +++ b/openmc/lib/tally.py @@ -402,7 +402,7 @@ class _TallyMapping(Mapping): def __repr__(self): return repr(dict(self)) - def __delitem__(self,key): + def __delitem__(self, key): """Delete a tally from tally vector and remove the ID,index pair from tally""" _dll.openmc_remove_tally(self._get_tally_index(key)) From d69a6ec647d7f78eac7cf25586df9ab2ae2dec41 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Mon, 15 Aug 2022 13:43:53 -0500 Subject: [PATCH 25/32] [skip ci] erasing the tally from the tallies vector calls the destructor, which already handles removing this tally from the map --- src/tallies/tally.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tallies/tally.cpp b/src/tallies/tally.cpp index f45fae994..59adce455 100644 --- a/src/tallies/tally.cpp +++ b/src/tallies/tally.cpp @@ -1281,8 +1281,8 @@ extern "C" int openmc_remove_tally(int32_t index) } // grab tally so it's ID can be obtained to remove the (ID,index) pair from tally_map auto& tally = model::tallies[index]; - model::tally_map.erase(tally->id_); // delete the tally via iterator pointing to correct position + // this calls the Tally destructor, removing the tally from the map as well model::tallies.erase(model::tallies.begin() + index); return 0; From 7fdd96a5734d2accedabcd1b6c0397230b3a04c3 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Mon, 15 Aug 2022 13:46:26 -0500 Subject: [PATCH 26/32] forgot a space for formatting requirements --- openmc/lib/tally.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmc/lib/tally.py b/openmc/lib/tally.py index 0be2f9ed5..79f1193e6 100644 --- a/openmc/lib/tally.py +++ b/openmc/lib/tally.py @@ -406,7 +406,7 @@ class _TallyMapping(Mapping): """Delete a tally from tally vector and remove the ID,index pair from tally""" _dll.openmc_remove_tally(self._get_tally_index(key)) - def _get_tally_index(self,key): + def _get_tally_index(self, key): """Given the ID, return the index""" index = c_int32() try: From 15a863e6457766ae792ca44e4b706060318e85f4 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Mon, 15 Aug 2022 14:49:36 -0500 Subject: [PATCH 27/32] suggestion by Paul Romano. removed need for _get_tally_index_ funciton by using python's fancy self[key] and self[key]._index capabilities --- openmc/lib/tally.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/openmc/lib/tally.py b/openmc/lib/tally.py index 79f1193e6..90983d171 100644 --- a/openmc/lib/tally.py +++ b/openmc/lib/tally.py @@ -390,7 +390,7 @@ class Tally(_FortranObjectWithID): class _TallyMapping(Mapping): def __getitem__(self, key): - return Tally(index=self._get_tally_index(key)) + return Tally(index=self[key]) def __iter__(self): for i in range(len(self)): @@ -404,16 +404,6 @@ class _TallyMapping(Mapping): def __delitem__(self, key): """Delete a tally from tally vector and remove the ID,index pair from tally""" - _dll.openmc_remove_tally(self._get_tally_index(key)) - - def _get_tally_index(self, key): - """Given the ID, return the index""" - index = c_int32() - try: - _dll.openmc_get_tally_index(key, index) - except (AllocationError, InvalidIDError) as e: - # __contains__ expects a KeyError to work correctly - raise KeyError(str(e)) - return index.value + _dll.openmc_remove_tally(self[key]._index) tallies = _TallyMapping() From 209954ee2fb652db70b84596b5accd35afbb87bb Mon Sep 17 00:00:00 2001 From: Lewis Gross <43077972+lewisgross1296@users.noreply.github.com> Date: Tue, 16 Aug 2022 13:08:35 -0500 Subject: [PATCH 28/32] change back to prevent accidental infinite recursion Co-authored-by: Patrick Shriwise --- openmc/lib/tally.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/openmc/lib/tally.py b/openmc/lib/tally.py index 90983d171..dc782539b 100644 --- a/openmc/lib/tally.py +++ b/openmc/lib/tally.py @@ -390,7 +390,12 @@ class Tally(_FortranObjectWithID): class _TallyMapping(Mapping): def __getitem__(self, key): - return Tally(index=self[key]) + index = c_int32() + try: + _dll.openmc_get_tally_index(key, index) + except (AllocationError, InvalidIDError) as e: + # __contains__ expects a KeyError to work correctly + raise KeyError(str(e)) def __iter__(self): for i in range(len(self)): From 5b6c6667cbc082d438ea38708fd4da34b89b1a18 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Tue, 16 Aug 2022 13:43:15 -0500 Subject: [PATCH 29/32] put _get_tally_index(self,key) back and forgot return in __getitem__ --- openmc/lib/tally.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/openmc/lib/tally.py b/openmc/lib/tally.py index dc782539b..5ddee90e4 100644 --- a/openmc/lib/tally.py +++ b/openmc/lib/tally.py @@ -396,6 +396,7 @@ class _TallyMapping(Mapping): except (AllocationError, InvalidIDError) as e: # __contains__ expects a KeyError to work correctly raise KeyError(str(e)) + return Tally(index=index.value) def __iter__(self): for i in range(len(self)): @@ -411,4 +412,14 @@ class _TallyMapping(Mapping): """Delete a tally from tally vector and remove the ID,index pair from tally""" _dll.openmc_remove_tally(self[key]._index) + def _get_tally_index(self,key): + """Given the ID, return the index""" + index = c_int32() + try: + _dll.openmc_get_tally_index(key, index) + except (AllocationError, InvalidIDError) as e: + # __contains__ expects a KeyError to work correctly + raise KeyError(str(e)) + return index.value + tallies = _TallyMapping() From a8b0f1714af39b2803055c6dc8288f22739bd8f2 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Tue, 16 Aug 2022 14:09:05 -0500 Subject: [PATCH 30/32] mistaken, dont need _get_tally_index, we call the openmc C++ api one instead --- openmc/lib/tally.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/openmc/lib/tally.py b/openmc/lib/tally.py index 5ddee90e4..85af8ffff 100644 --- a/openmc/lib/tally.py +++ b/openmc/lib/tally.py @@ -412,14 +412,4 @@ class _TallyMapping(Mapping): """Delete a tally from tally vector and remove the ID,index pair from tally""" _dll.openmc_remove_tally(self[key]._index) - def _get_tally_index(self,key): - """Given the ID, return the index""" - index = c_int32() - try: - _dll.openmc_get_tally_index(key, index) - except (AllocationError, InvalidIDError) as e: - # __contains__ expects a KeyError to work correctly - raise KeyError(str(e)) - return index.value - tallies = _TallyMapping() From 37b17b61cf55cba9158528d968ab1332ec602548 Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Tue, 16 Aug 2022 14:26:48 -0500 Subject: [PATCH 31/32] added openmc_remove_tally to documentation --- docs/source/capi/index.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/source/capi/index.rst b/docs/source/capi/index.rst index 222f81c4b..623c9fa5c 100644 --- a/docs/source/capi/index.rst +++ b/docs/source/capi/index.rst @@ -460,6 +460,14 @@ Functions :return: Return status (negative if an error occurs) :rtype: int +.. c:function:: int openmc_remove_tally(int32_t index); + + Given an index of a tally, remove it from the tallies vector + :param int index: Index in tallies vector + :return: Return status (negative if an error occurs) + :rtype: int + + .. c:function:: int openmc_run() Run a simulation From 5003cf482361061d35b0195166f6c87580571eba Mon Sep 17 00:00:00 2001 From: lewisgross1296 Date: Tue, 16 Aug 2022 15:42:45 -0500 Subject: [PATCH 32/32] changed vector to array to match other entries, removed extra trailing space --- docs/source/capi/index.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/source/capi/index.rst b/docs/source/capi/index.rst index 623c9fa5c..52c0b2406 100644 --- a/docs/source/capi/index.rst +++ b/docs/source/capi/index.rst @@ -462,12 +462,11 @@ Functions .. c:function:: int openmc_remove_tally(int32_t index); - Given an index of a tally, remove it from the tallies vector - :param int index: Index in tallies vector + Given an index of a tally, remove it from the tallies array + :param int index: Index in tallies array :return: Return status (negative if an error occurs) :rtype: int - .. c:function:: int openmc_run() Run a simulation