TotalInvTracker to be an inventory check on whole agents

[nuclearkatie]

TotalInvTracker keeps track of multiple ResBufs.

• Has its own maximum inventory.
• Checking for the space available in a TotalInvTracker returns the min of its own inventory and the space remaining in all ResBufs
• Its perfectly acceptable to build a tracker with only one ResBuf
• MatlBuyPolicy modified to require a TotalInvTracker. This allows there to be a total agent cap in addition to any restrictions placed on the receiving buffer connected to the buy policy
  • Importantly, this will allow for cycamore:Storage to be fixed (see cycamore#554). Currently it abuses its receiver ResBuf inventory by constantly modifying its total allowable inventory. The new process will be to set a receiving ResBuf with a max inventory (and throughput, etc) and then set up a TotalInvTracker that is aware of all ResBufs in the archetype and has its own maximum inventory. This way, the capacity of inventory will never have to change, but material present in other ResBufs will restrict the demand against the total allowable inventory

(R,Q): Closes #1647, closes #1648, closes #1649
(s:S): closes #1650, closes #1651, closes #1652

[coveralls]

Pull Request Test Coverage Report for Build 7794843344

• -17 of 119 (85.71%) changed or added relevant lines in 3 files are covered.
• 20 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-0.005%) to 49.331%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/toolkit/matl_buy_policy.cc	75	83	90.36%
src/toolkit/total_inv_tracker.h	20	29	68.97%

Files with Coverage Reduction	New Missed Lines	%
src/pyinfile.cc	3	48.57%
src/toolkit/matl_buy_policy.cc	17	51.59%

Totals
Change from base Build 7027753240:	-0.005%
Covered Lines:	24298
Relevant Lines:	41685

---

💛 - Coveralls

[abachma2]

This looks pretty good to me. I have a few small questions for you. Would any of this information show up in the output database anywhere? Or is this more for internal cyclus record keeping? I don't think I saw anything about writing to the database.

[nuclearkatie]

Thanks @abachma2. Fair point about the database. Right now, the execution of these buy policies only shows up in the AgentState info (and the copy of the input file, ofc), and then indirectly through the demand raised by the agents implementing that policy (which is aggregated if multiple agents demand the same commodity). I did include some logging statements.

I guess, if you were using one of these policies, what would you want to see? Something like an optional table (like explicit_inventory?) that records additional info about the demand and if its affected by one of the policies I've developed (these or the active/dormant policies from #1634) ?

[abachma2]

I'm not sure if I would necessarily need something to be in the output, I feel like explicit_inventory would capture this some since the total inventory is limited. I was mostly doing a sanity check based on your code.

[gonuke]

I don't think anything about the current Storage class shows up in the database either, other than the total storage size as an agent state variable

[gonuke]

This has a lot of interesting things going on @nuclearkatie - thanks for this effort.

Some of these comments connect to larger questions about the code design of the MatBuyPolicy as it gets more complex. In particular, we should discuss the different initializations that are possible and whether some of them are replaced by explicit calls to setup policy parameters outside of an Init() call?

This review covers much of the changes, but I'm holding off on reviewing tests until we resolve some of these other questions.

[gonuke]

As all this logic has become more complicated, I'm thinking about a system that separates out the different purposes into different chunks or code:

• determine if the inventory policy requires/allows a request to be made
• determine if the time requires/allows a request to be made
• update the next active/dormant times

It may cause some redundancy in the checks that occur, but it will be easier to know what is happening everyhere.

[nuclearkatie]

Is this more clear?

cyclus/src/toolkit/matl_buy_policy.cc

Lines 265 to 307 in c4ac399

	// Step 1: determine if inventory policy allows for a request to be made
	// Handing for (s,S)/(R,Q) inventory policies
	if (!MakeReq()) {
	return ports;
	}
	// Handling for cumulative capacity inventory policy
	// buy while not in dormant, don't buy while dormant. Make sure the request
	// is less than or equal to the space remaining in the cycle capacity
	// (cumulative cap minus current cycle inventory)
	if (use_cumulative_capacity()) {
	if (no_cycle_end_time() || current_time_ == next_dormant_end_){
	amt = std::min((TotalAvailable() * SampleRequestSize()),
	(cumulative_cap_ - cycle_total_inv_));
	}
	else if (current_time_ < next_dormant_end_) {
	amt = 0;
	LGH(INFO3) << "in dormant period, no request" << std::endl;
	}
	}
	// Step 2: determine if active/dormant cycle times allow for a request
	// if no cycles, or if in the middle of active period, or if dormant period
	// just ended, then request
	else if (no_cycle_end_time() || (current_time_ < next_active_end_) ||
	(current_time_ == next_dormant_end_)) {
	amt = TotalAvailable() * SampleRequestSize();
	}
	// if in the middle of dormant period, then don't request
	else if (current_time_ < next_dormant_end_) {
	amt = 0;
	LGH(INFO3) << "in dormant period, no request" << std::endl;
	}
	// Step 3: Finally, determine if active/dormant cycle times need to be reset.
	// If reaching the end of a cumulative cap cycle, set next_dormant_end_ = -1,
	// otherwise sample for next dormant.
	if (current_time_ == next_dormant_end_) {
	SetNextActiveTime();
	if (use_cumulative_capacity()) { next_dormant_end_ = -1; }
	else {
	SetNextDormantTime();
	LGH(INFO4) << "end of dormant period, next active time end: " << next_active_end_ << ", and next dormant time end: " << next_dormant_end_ << std::endl;
	}
	}

[gonuke]

Nearly there.... I think the logic is more clear now, but still very confusing to follow when certain conditions constrain things.

I'm thinking it might make sense to have some local state variables:

// pseudo-code
double amt = 0;
double max_request_amt = using_cummulative_capacity() ? cumulative_capacity_ - cycle_total_inv_ : max_double;

if (MakeReq() && !dormant()) amt = std::min( TotalAvailable() * SampleRequestSize(), max_request_amt);

resetActiveDormantCumulative();

[gonuke]

A few more tweaks for robustness and long-term readability

[gonuke]

Thanks for this big addition @nuclearkatie

[gonuke]

Merging now because the cycamore update is close to ready