Correction to deployment scheme

[abachma2]

This PR fixes an issue observed after #141 was merged that was not caught in the test suite. With this PR the deployment scheme is as follows:

• when there is capacity to be installed from an increase in the demand or from decommissioning of facilities in another institution:
  • The prototypes are preferentially deployed based on power
  • The prototype with the largest capacity is deployed until an oversupply of energy would be created (round down on the number deployed)
  • The other prototypes are deployed in a similar manner until the smallest prototype is deployed
  • The smallest prototype is deployed until the demand is at least met (round up on the number deployed)
  • If there is a specified build share for one or more prototypes, then the prototypes with a specified build share are deployed first, and then the other prototypes are deployed based on the previous 4 points, with the prototypes with a specified build share removed from the deployment order (the list of prototypes in order of decreasing capacity)
• when there is capacity to be installed from the decommissioning of prototypes in this institution:
  • It is assumed first that the number of decommissioning prototypes are re-deployed (e.g. look at time step current minus the lifetime of the prototype in question)
  • If fewer prototypes can be deployed while still meeting the demand, such as in the case of a decrease in the demand, then the lesser number of prototypes are deployed.

This PR fixes the issue of large variations in the number of prototypes re-deployed if multiple prototypes were decommissioned at the same time step.

All tests currently pass. I encourage you to ensure that the test suite for these functions is comprehensive, as the error this fixes was not previously captured in the test suite.

[samgdotson]

I'm requesting some changes to make the code more readable. I generally prefer longer variable names to shorter ones (although I'm definitely guilty of making lazy names). rx is not a helpful shortening of reactor in my opinion. There are several places where an array slice is extremely verbose (e.g. power_gap[index:index+reactor_prototypes[prototype][1]]) which makes it hard to interpret what's happening.

I'm a little bit surprised that .circleci didn't run this PR. But several previous builds have failed -- you may want to check why that is.

[abachma2]

Hi @samgdotson, thanks for the review. I think your comments help with the readability of the code. For the CircleCI, it is a known issue with that (Issue #128) because there are some issues with building pyne. I haven't gotten around to fixing it.

[samgdotson]

I made a suggestion, but the text of the comment had what I actually wanted. I couldn’t make it a github suggestion because it includes lines that weren’t changed in the PR.

On Wed, Oct 26, 2022 at 11:06 AM Amanda Bachmann ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In scripts/create_AR_DeployInst.py <https://urldefense.com/v3/__https://github.com/arfc/transition-scenarios/pull/143*discussion_r1005812613__;Iw!!DZ3fjg!7KjZZbsWY0bQ5tTMdZETnnewVSX2-gpzF4Occ-SlvnQ_qGe0G4Ds1AhYBakNJIC_M_6LLR5wrrPpCSHwLPCrmUvIBA$> : > + [1]] = power_gap[index:index+reactor_prototypes[prototype] + [1]] - reactor_prototypes[prototype][0] * num_rxs Did I not? I thought I made the change you requested. — Reply to this email directly, view it on GitHub <https://urldefense.com/v3/__https://github.com/arfc/transition-scenarios/pull/143*discussion_r1005812613__;Iw!!DZ3fjg!7KjZZbsWY0bQ5tTMdZETnnewVSX2-gpzF4Occ-SlvnQ_qGe0G4Ds1AhYBakNJIC_M_6LLR5wrrPpCSHwLPCrmUvIBA$>, or unsubscribe <https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AKSJ4WOOZO44ZXBQUXVO6ODWFFCG5ANCNFSM6AAAAAARNFVA6M__;!!DZ3fjg!7KjZZbsWY0bQ5tTMdZETnnewVSX2-gpzF4Occ-SlvnQ_qGe0G4Ds1AhYBakNJIC_M_6LLR5wrrPpCSHwLPBjCmJ30w$> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Samuel G. Dotson Master of Science in Nuclear Engineering University of Illinois at Urbana-Champaign Pronouns: he/him/his ***@***.***

[samgdotson]

There are a few remaining things that need to be changed in your test file. I made them suggestions so it should be easy to rectify.

As a matter of personal preference, every instance of rx in your create_AR_DeployInst script should be replaced by reactor. I don't know what IDE you use, but most of them have a "find and replace all" feature. If you handle the other comments but not this one, I will still merge.

[abachma2]

Thanks for catching those @samgdotson! I have made the requested changes.

[samgdotson]

Looks good @abachma2, merging.