From 163e6c79857750237e43e3cded0253d685fb01fd Mon Sep 17 00:00:00 2001 From: Katie Mummah Date: Tue, 3 Sep 2024 14:45:21 -0500 Subject: [PATCH 1/6] change recording for 1:1 extracted resources --- CHANGELOG.rst | 2 +- src/material.cc | 5 +---- src/res_tracker.cc | 17 +++++++++++------ src/res_tracker.h | 2 +- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 938fd718ec..dd7725129a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -18,7 +18,7 @@ Since last release * Rely on ``python3`` in environment instead of ``python`` (#1747) * Remove ``pandas`` as build dependency (#1748) * Consistently use hyphens in ``install.py`` flags (#1748) -* Material sell policy can package materials (#1749, #1774, #1775) +* Material sell policy can package materials (#1749, #1774, #1775, #1795) * Use miniforge for conda CI builds instead of miniconda (#1763) * Define constants ``CY_LARGE_DOUBLE``, ``CY_LARGE_INT``, and ``CY_NEAR_ZERO`` (#1757) * Warning and limits on number of packages that can be created from a resource at once (#1771) diff --git a/src/material.cc b/src/material.cc index 186674ea70..db8b7e3b74 100644 --- a/src/material.cc +++ b/src/material.cc @@ -93,10 +93,7 @@ Material::Ptr Material::ExtractComp(double qty, Composition::Ptr c, // this material regardless of composition. other->prev_decay_time_ = prev_decay_time_; - if (qty_ > cyclus::eps()) { - tracker_.Extract(&other->tracker_); - } // else, this material is being fully extracted and nothing has effectively - // changed. Don't need to bump state, parent, etc. + tracker_.Extract(&other->tracker_); return other; } diff --git a/src/res_tracker.cc b/src/res_tracker.cc index d9c877bab0..f11b174be7 100644 --- a/src/res_tracker.cc +++ b/src/res_tracker.cc @@ -50,11 +50,15 @@ void ResTracker::Extract(ResTracker* removed) { parent2_ = 0; Record(); + + removed->parent1_ = res_->state_id(); + removed->parent2_ = 0; + removed->tracked_ = tracked_; + } else { + removed->parent1_ = parent1_; + removed->parent2_ = parent2_; + removed->tracked_ = tracked_; } - - removed->parent1_ = res_->state_id(); - removed->parent2_ = 0; - removed->tracked_ = tracked_; removed->Record(); } @@ -78,7 +82,8 @@ void ResTracker::Package() { Record(); } -void ResTracker::Record() { +void ResTracker::Record(bool no_bump) { + if (!no_bump) { res_->BumpStateId(); ctx_->NewDatum("Resources") ->AddVal("ResourceId", res_->state_id()) @@ -92,7 +97,7 @@ void ResTracker::Record() { ->AddVal("Parent1", parent1_) ->AddVal("Parent2", parent2_) ->Record(); - + } res_->Record(ctx_); } diff --git a/src/res_tracker.h b/src/res_tracker.h index c4e83b71e5..d0fa6372ce 100644 --- a/src/res_tracker.h +++ b/src/res_tracker.h @@ -46,7 +46,7 @@ class ResTracker { void Package(); private: - void Record(); + void Record(bool no_bump = false); int parent1_; int parent2_; From 14aea99e605d8fd6832f758b41b41a6c0a90fa1d Mon Sep 17 00:00:00 2001 From: Katie Mummah Date: Wed, 4 Sep 2024 16:48:19 -0500 Subject: [PATCH 2/6] specialized extract for packaging --- src/material.cc | 21 ++++++++++++++++++--- src/material.h | 3 +++ src/product.cc | 12 ++++++++++++ src/product.h | 2 ++ src/res_tracker.cc | 10 ++-------- src/res_tracker.h | 2 +- src/resource.h | 11 +++++++++-- 7 files changed, 47 insertions(+), 14 deletions(-) diff --git a/src/material.cc b/src/material.cc index db8b7e3b74..63b1c5b157 100644 --- a/src/material.cc +++ b/src/material.cc @@ -140,15 +140,30 @@ void Material::Transmute(Composition::Ptr c) { } } +Resource::Ptr Material::PackageExtract(double qty, std::string new_package_name) { + if (qty > qty_) { + throw ValueError("Attempted to extract more quantity than exists."); + } + + qty_ -= qty; + Material::Ptr other(new Material(ctx_, qty, comp_, new_package_name)); + + // Decay called on the extracted material should have the same dt as for + // this material regardless of composition. + other->prev_decay_time_ = prev_decay_time_; + + tracker_.Extract(&other->tracker_); + return boost::static_pointer_cast(other); +} + void Material::ChangePackage(std::string new_package_name) { - if (new_package_name == package_name_ || ctx_ == NULL) { + if (ctx_ == NULL) { // no change needed return; } else if (new_package_name == Package::unpackaged_name()) { // unpackaged has functionally no restrictions package_name_ = new_package_name; - tracker_.Package(); return; } @@ -157,10 +172,10 @@ void Material::ChangePackage(std::string new_package_name) { double max = p->fill_max(); if (qty_ >= min && qty_ <= max) { package_name_ = new_package_name; - tracker_.Package(); } else { throw ValueError("Material quantity is outside of package fill limits."); } + tracker_.Package(); } void Material::Decay(int curr_time) { diff --git a/src/material.h b/src/material.h index 008f9fde06..7d08eae319 100644 --- a/src/material.h +++ b/src/material.h @@ -158,6 +158,9 @@ class Material: public Resource { virtual std::string package_name(); + virtual Resource::Ptr PackageExtract(double qty, + std::string new_package_name = Package::unpackaged_name()); + /// Changes the package id. Checks that the resource fits the package /// type minimum and maximum mass criteria. virtual void ChangePackage(std::string new_package_name = Package::unpackaged_name()); diff --git a/src/product.cc b/src/product.cc index 254624ca1b..f07fb73cd7 100644 --- a/src/product.cc +++ b/src/product.cc @@ -76,6 +76,18 @@ std::string Product::package_name() { return package_name_; } +Resource::Ptr Product::PackageExtract(double qty, std::string new_package_name) { + if (qty > quantity_) { + throw ValueError("Attempted to extract more quantity than exists."); + } + + quantity_ -= qty; + Product::Ptr other(new Product(ctx_, qty, quality_, new_package_name)); + + tracker_.Extract(&other->tracker_); + return boost::static_pointer_cast(other); +} + void Product::ChangePackage(std::string new_package_name) { if (new_package_name == package_name_ || ctx_ == NULL) { // no change needed diff --git a/src/product.h b/src/product.h index 99f4091d69..68d734093b 100644 --- a/src/product.h +++ b/src/product.h @@ -74,6 +74,8 @@ class Product : public Resource { /// Returns the package id. virtual std::string package_name(); + virtual Resource::Ptr PackageExtract(double qty, std::string new_package_name = Package::unpackaged_name()); + /// Changes the product's package id virtual void ChangePackage(std::string new_package_name = Package::unpackaged_name()); diff --git a/src/res_tracker.cc b/src/res_tracker.cc index f11b174be7..159fca7daf 100644 --- a/src/res_tracker.cc +++ b/src/res_tracker.cc @@ -50,15 +50,11 @@ void ResTracker::Extract(ResTracker* removed) { parent2_ = 0; Record(); + } removed->parent1_ = res_->state_id(); removed->parent2_ = 0; removed->tracked_ = tracked_; - } else { - removed->parent1_ = parent1_; - removed->parent2_ = parent2_; - removed->tracked_ = tracked_; - } removed->Record(); } @@ -82,8 +78,7 @@ void ResTracker::Package() { Record(); } -void ResTracker::Record(bool no_bump) { - if (!no_bump) { +void ResTracker::Record() { res_->BumpStateId(); ctx_->NewDatum("Resources") ->AddVal("ResourceId", res_->state_id()) @@ -97,7 +92,6 @@ void ResTracker::Record(bool no_bump) { ->AddVal("Parent1", parent1_) ->AddVal("Parent2", parent2_) ->Record(); - } res_->Record(ctx_); } diff --git a/src/res_tracker.h b/src/res_tracker.h index d0fa6372ce..c4e83b71e5 100644 --- a/src/res_tracker.h +++ b/src/res_tracker.h @@ -46,7 +46,7 @@ class ResTracker { void Package(); private: - void Record(bool no_bump = false); + void Record(); int parent1_; int parent2_; diff --git a/src/resource.h b/src/resource.h index 919a36a323..f4d79659a2 100644 --- a/src/resource.h +++ b/src/resource.h @@ -94,6 +94,8 @@ class Resource { /// Returns the package id. virtual std::string package_name() { return Package::unpackaged_name(); }; + virtual Ptr PackageExtract(double qty, std::string new_package_name = Package::unpackaged_name()) = 0; + /// Changes the product's package id virtual void ChangePackage(std::string new_package_name = Package::unpackaged_name()) {}; @@ -107,6 +109,12 @@ class Resource { static int nextstate_id_; static int nextobj_id_; int state_id_; + // Setting the state id should only be done when extracting one resource + void state_id(int st_id) { + state_id_ = st_id; + } + + int obj_id_; }; @@ -145,8 +153,7 @@ std::vector Resource::Package(Package::Ptr pkg) { while (quantity() > 0 && quantity() >= pkg->fill_min()) { double pkg_fill = std::min(quantity(), fill_mass); - t_pkgd = boost::dynamic_pointer_cast(ExtractRes(pkg_fill)); - t_pkgd->ChangePackage(pkg->name()); + t_pkgd = boost::dynamic_pointer_cast(PackageExtract(pkg_fill, pkg->name())); ts_pkgd.push_back(t_pkgd); } return ts_pkgd; From 7335591099ae447c7c9791916d1abcf6e4f80099 Mon Sep 17 00:00:00 2001 From: Katie Mummah Date: Thu, 5 Sep 2024 13:48:53 -0500 Subject: [PATCH 3/6] bump state id only if resource is not new, if new it already has a new stateid --- src/material.cc | 7 ++++++- src/product.cc | 8 +++++++- src/res_tracker.cc | 32 ++++++++++++++++++++++++++------ src/res_tracker.h | 9 ++++++--- 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/material.cc b/src/material.cc index 63b1c5b157..17301ca7f4 100644 --- a/src/material.cc +++ b/src/material.cc @@ -152,7 +152,12 @@ Resource::Ptr Material::PackageExtract(double qty, std::string new_package_name) // this material regardless of composition. other->prev_decay_time_ = prev_decay_time_; - tracker_.Extract(&other->tracker_); + // this call to res_tracker must come first before the parent resource + // state id gets modified + other->tracker_.Package(&tracker_); + if (qty_ > cyclus::eps_rsrc()) { + tracker_.Modify(); + } return boost::static_pointer_cast(other); } diff --git a/src/product.cc b/src/product.cc index f07fb73cd7..3081512c9b 100644 --- a/src/product.cc +++ b/src/product.cc @@ -2,6 +2,7 @@ #include "error.h" #include "logger.h" +#include "cyc_limits.h" namespace cyclus { @@ -84,7 +85,12 @@ Resource::Ptr Product::PackageExtract(double qty, std::string new_package_name) quantity_ -= qty; Product::Ptr other(new Product(ctx_, qty, quality_, new_package_name)); - tracker_.Extract(&other->tracker_); + // this call to res_tracker must come first before the parent resource + // state id gets modified + other->tracker_.Package(&tracker_); + if (quantity_ > cyclus::eps_rsrc()) { + tracker_.Modify(); + } return boost::static_pointer_cast(other); } diff --git a/src/res_tracker.cc b/src/res_tracker.cc index 159fca7daf..25f87e8e79 100644 --- a/src/res_tracker.cc +++ b/src/res_tracker.cc @@ -23,7 +23,8 @@ void ResTracker::Create(Agent* creator) { parent1_ = 0; parent2_ = 0; - Record(); + bool no_bump = true; + Record(no_bump); ctx_->NewDatum("ResCreators") ->AddVal("ResourceId", res_->state_id()) ->AddVal("AgentId", creator->id()) @@ -69,17 +70,36 @@ void ResTracker::Absorb(ResTracker* absorbed) { Record(); } -void ResTracker::Package() { +void ResTracker::Package(ResTracker* parent) { if (!tracked_) { return; } - + parent2_ = 0; + tracked_ = tracked_; package_name_ = res_->package_name(); - Record(); + + if (parent != NULL) { + parent1_ = parent->res_->state_id(); + + // Resource was just created, with packaging info, and assigned a state id. + // Do not need to bump again + bool no_bump = true; + Record(no_bump); + } else { + // Resource was not just created. It is being re-packaged. It needs to be + // bumped to get a new state id. + parent1_ = res_->state_id(); + Record(); + } + + + } -void ResTracker::Record() { - res_->BumpStateId(); +void ResTracker::Record(bool no_bump) { + if (!no_bump) { + res_->BumpStateId(); + } ctx_->NewDatum("Resources") ->AddVal("ResourceId", res_->state_id()) ->AddVal("ObjId", res_->obj_id()) diff --git a/src/res_tracker.h b/src/res_tracker.h index c4e83b71e5..81f5ddbb96 100644 --- a/src/res_tracker.h +++ b/src/res_tracker.h @@ -42,11 +42,14 @@ class ResTracker { /// decay). void Modify(); - /// Should be called when a resource's package gets modified - void Package(); + /// Should be called when a resource's package gets modified. If the resource + /// was just created from a parent resource, the parent should be passed in. + /// If the resource is just being repackaged (e.g. to unpackaged), the parent + /// should be NULL. + void Package(ResTracker* parent = NULL); private: - void Record(); + void Record(bool no_bump = false); int parent1_; int parent2_; From 45e8af912331a6a37fb0c405b34e151dd5805689 Mon Sep 17 00:00:00 2001 From: Katie Mummah Date: Wed, 11 Sep 2024 09:58:34 -0500 Subject: [PATCH 4/6] always check quantity against eps_rsrc() instead of zero --- src/material.cc | 4 ++-- src/package.cc | 3 ++- src/resource.h | 3 ++- src/toolkit/matl_buy_policy.cc | 2 +- src/toolkit/matl_sell_policy.cc | 4 ++-- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/material.cc b/src/material.cc index 17301ca7f4..9a80b075d8 100644 --- a/src/material.cc +++ b/src/material.cc @@ -141,7 +141,7 @@ void Material::Transmute(Composition::Ptr c) { } Resource::Ptr Material::PackageExtract(double qty, std::string new_package_name) { - if (qty > qty_) { + if ((qty - qty_) > eps_rsrc()) { throw ValueError("Attempted to extract more quantity than exists."); } @@ -155,7 +155,7 @@ Resource::Ptr Material::PackageExtract(double qty, std::string new_package_name) // this call to res_tracker must come first before the parent resource // state id gets modified other->tracker_.Package(&tracker_); - if (qty_ > cyclus::eps_rsrc()) { + if (qty_ > eps_rsrc()) { tracker_.Modify(); } return boost::static_pointer_cast(other); diff --git a/src/package.cc b/src/package.cc index 8f5b9f3a7a..b5ab64e301 100644 --- a/src/package.cc +++ b/src/package.cc @@ -1,5 +1,6 @@ #include "package.h" #include "error.h" +#include "cyc_limits.h" namespace cyclus { @@ -30,7 +31,7 @@ Package::Ptr& Package::unpackaged() { } std::pair Package::GetFillMass(double qty) { - if (qty < fill_min_) { + if ((qty - fill_min_) < -eps_rsrc()) { // less than one pkg of material available return std::pair (0, 0); } diff --git a/src/resource.h b/src/resource.h index f4d79659a2..d614f86b29 100644 --- a/src/resource.h +++ b/src/resource.h @@ -6,6 +6,7 @@ #include #include "package.h" +#include "cyc_limits.h" class SimInitTest; @@ -151,7 +152,7 @@ std::vector Resource::Package(Package::Ptr pkg) { int approx_num_pkgs = fill.second; Package::ExceedsSplitLimits(approx_num_pkgs); - while (quantity() > 0 && quantity() >= pkg->fill_min()) { + while (quantity() > 0 && (quantity() - pkg->fill_min()) >= -eps_rsrc()) { double pkg_fill = std::min(quantity(), fill_mass); t_pkgd = boost::dynamic_pointer_cast(PackageExtract(pkg_fill, pkg->name())); ts_pkgd.push_back(t_pkgd); diff --git a/src/toolkit/matl_buy_policy.cc b/src/toolkit/matl_buy_policy.cc index a00a6e0658..5db7fc4519 100644 --- a/src/toolkit/matl_buy_policy.cc +++ b/src/toolkit/matl_buy_policy.cc @@ -318,7 +318,7 @@ void MatlBuyPolicy::AcceptMatlTrades( // check if cumulative cap has been reached. If yes, then sample for dormant // length and reset cycle_total_inv if (use_cumulative_capacity() && ( - (cumulative_cap_ - cycle_total_inv_) < eps())) { + (cumulative_cap_ - cycle_total_inv_) < eps_rsrc())) { SetNextDormantTime(); LGH(INFO3) << "cycle cumulative inventory has been reached. Dormant period will end at " << next_dormant_end_ << std::endl; cycle_total_inv_ = 0; diff --git a/src/toolkit/matl_sell_policy.cc b/src/toolkit/matl_sell_policy.cc index 603b7b558c..50e6105e18 100644 --- a/src/toolkit/matl_sell_policy.cc +++ b/src/toolkit/matl_sell_policy.cc @@ -265,7 +265,7 @@ void MatlSellPolicy::GetMatlTrades( if (shippable_pkgs > 0){ qty = it->amt; LGH(INFO3) << " sending " << qty << " kg of " << it->request->commodity(); - Material::Ptr mat = buf_->Pop(qty, cyclus::eps_rsrc()); + Material::Ptr mat = buf_->Pop(qty, eps_rsrc()); Material::Ptr trade_mat; // don't go through packaging if you don't need to. packaging always bumps @@ -274,7 +274,7 @@ void MatlSellPolicy::GetMatlTrades( if (package_->name() != mat->package_name()) { // packaging needed std::vector mat_pkgd = mat->Package(package_); - if (mat->quantity() > eps()) { + if (mat->quantity() > eps_rsrc()) { // push any extra material that couldn't be packaged back onto buffer // don't push unless there's leftover material buf_->Push(mat); From 8abc7a710efec0e3327b296cde6183224236cfc4 Mon Sep 17 00:00:00 2001 From: Katie Mummah Date: Thu, 12 Sep 2024 09:39:26 -0600 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Paul Wilson Signed-off-by: Katie Mummah --- src/res_tracker.cc | 6 +++--- src/res_tracker.h | 2 +- src/resource.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/res_tracker.cc b/src/res_tracker.cc index 25f87e8e79..72b9304229 100644 --- a/src/res_tracker.cc +++ b/src/res_tracker.cc @@ -53,9 +53,9 @@ void ResTracker::Extract(ResTracker* removed) { Record(); } - removed->parent1_ = res_->state_id(); - removed->parent2_ = 0; - removed->tracked_ = tracked_; + removed->parent1_ = res_->state_id(); + removed->parent2_ = 0; + removed->tracked_ = tracked_; removed->Record(); } diff --git a/src/res_tracker.h b/src/res_tracker.h index 81f5ddbb96..ce650bf6b4 100644 --- a/src/res_tracker.h +++ b/src/res_tracker.h @@ -49,7 +49,7 @@ class ResTracker { void Package(ResTracker* parent = NULL); private: - void Record(bool no_bump = false); + void Record(bool bumpId = true); int parent1_; int parent2_; diff --git a/src/resource.h b/src/resource.h index d614f86b29..aa29a7617b 100644 --- a/src/resource.h +++ b/src/resource.h @@ -95,7 +95,7 @@ class Resource { /// Returns the package id. virtual std::string package_name() { return Package::unpackaged_name(); }; - virtual Ptr PackageExtract(double qty, std::string new_package_name = Package::unpackaged_name()) = 0; + virtual Ptr PackageExtract(double qty, std::string new_package_name) = 0; /// Changes the product's package id virtual void ChangePackage(std::string new_package_name = Package::unpackaged_name()) {}; From ac72e6a3324372d13cfd212eda6ac9f7e6db2963 Mon Sep 17 00:00:00 2001 From: Katie Mummah Date: Thu, 12 Sep 2024 10:53:41 -0500 Subject: [PATCH 6/6] update all no_bump to bumpId following code review --- src/res_tracker.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/res_tracker.cc b/src/res_tracker.cc index 72b9304229..244c497558 100644 --- a/src/res_tracker.cc +++ b/src/res_tracker.cc @@ -23,8 +23,8 @@ void ResTracker::Create(Agent* creator) { parent1_ = 0; parent2_ = 0; - bool no_bump = true; - Record(no_bump); + bool bumpId = false; + Record(bumpId); ctx_->NewDatum("ResCreators") ->AddVal("ResourceId", res_->state_id()) ->AddVal("AgentId", creator->id()) @@ -83,8 +83,8 @@ void ResTracker::Package(ResTracker* parent) { // Resource was just created, with packaging info, and assigned a state id. // Do not need to bump again - bool no_bump = true; - Record(no_bump); + bool bumpId = false; + Record(bumpId); } else { // Resource was not just created. It is being re-packaged. It needs to be // bumped to get a new state id. @@ -96,8 +96,8 @@ void ResTracker::Package(ResTracker* parent) { } -void ResTracker::Record(bool no_bump) { - if (!no_bump) { +void ResTracker::Record(bool bumpId) { + if (bumpId) { res_->BumpStateId(); } ctx_->NewDatum("Resources")