-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Source can package #613
Source can package #613
Conversation
Build Status Report - a9d8f20 - 2024-07-17 20:35:54 -0500Build
|
Might need to think about whether we want to tamp down on some of the automatic recording that happens during the packaging process. In my testing for this process, each trade package results in this many lines on a resource table:
I guess these all make sense, but they can be confusing because the amounts are the same. For example when the inventory is large, 1, 2, and 7 can appear identical (i.e. 1e299) even when they're not. When all of the popped resource can be packaged, 3, 4, and 5, and 6 are all the same quantity, with the only difference visible in a resource table being that 6 is finally labeled with the new package. My point is, a simple transaction has gone from 3 lines in a resource table (initial inventory, popped resource, and new inventory minus popped amount) to 7 lines in a way that might be confusing to users. Things are technically happening to trigger a new record in the resource table, but they're either happening behind the scenes, or they're not really changes at all (especially line 5). It was confusing to me the developer of packages and packaging in Source, and took me a little while to understand especially steps 3-6 |
cyclus#1775 addresses some of these issues. I also added a line to prevent active transmutation of the resource when there is no request composition or the request composition matches the existing transmutation. Previously, a new line in the resource database would be added if the agent had no |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few questions, some about clarity.
Did I merge too soon?? this is failing on merge even though tests passed on CI. I see now that it was failing with |
The failures on The failures on #8 1.188 [ RUN ] SourceTest.Package
#8 1.194 /cycamore/build/cycamore/source_tests.cc:187: Failure
#8 1.194 Expected equality of these values:
#8 1.194 qr_res.rows.size()
#8 1.194 Which is: 6
#8 1.194 3
#8 1.194
#8 1.196 [ FAILED ] SourceTest.Package (7 ms)
#8 1.196 [ RUN ] SourceTest.TransportUnit
#8 1.202 /cycamore/build/cycamore/source_tests.cc:219: Failure
#8 1.202 Expected equality of these values:
#8 1.202 qr_res.rows.size()
#8 1.202 Which is: 8
#8 1.202 4
#8 1.202
#8 1.204 [ FAILED ] SourceTest.TransportUnit (8 ms) @nuclearkatie do you have any insight to these failed tests? |
I'll take a look, I'm sure I made a mistake in building those tests somewhere. I was also confused by seeing CI checks passed but the comment showed latest failing |
See #617 |
Adds the ability for source to provide materials that are packaged.
Along the way, I also converted Source to a ResBuf facility. This means that the full inventory is created at the
EnterNotify
step and placed into the inventory buffer, then persists throughout the simulation. Previously, material never existed until the moment it was about to be traded, when it would be created. Source just maintained a number that represented the inventory at hand.This change is generally minor, but it makes packaging easier. Without pre-existing resources in a buffer, trades would be executed by created materials, trying to package them, and then in the rare case where the trade has some leftover material that can't be packaged (rare, and specifically only exists in cases where the requesting agent accepted a partial bid), then you're left with material that was already created but no buffer to place it back into..
This also allows sources to show up on explicit inventory tables, which is something I've wanted for a while anyway, so happy side benefit.