Skip to content
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

Update advanced reactor deployment scheme #141

Merged
merged 8 commits into from
Oct 20, 2022
Merged

Conversation

abachma2
Copy link
Contributor

I have updated the functions in scripts/create_AR_DeployInst.py to redeploy advanced reactors on a 1:1 basis. This is done by looking at the defined deployments at timestep current-lifetime of reactor. Then the number of reactors to redeploy is calculated by rounding up on the demand of energy at the current timestep divided by the energy output of the reactor in question. It is not a direct replacement of the same number of reactors to reduce any oversupply of power that may be caused.

Example:
4 of Type1 reactors are deployed at time 3, with a lifetime of 5 timesteps. They supply 100 MW for a total power supply of 500 MW against a demand of 480 MWe.

At timestep 8, a total of 410 MW are already deployed so only 3 Type1 reactors need to be deployed to meet the demand of 480 MW.

I have updated the tests for this module (scripts/tests/test_create_AR_DeployInst.py), and all tests pass.

Copy link
Member

@nsryan2 nsryan2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything seems to do what you want it to. My only bit of confusion was in the create_AR_DeployInst.py script with the three levels of nesting in the di dictionary (the last layer seems to be redundant, I could easily be not understanding that part though)

is 'DeployInst', the next level keys are 'prototypes',
'n_build', 'lifetimes', and 'build_times'. Each of the
second level keys have a nested dictionary of
'val':list
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'val':list
'val':list

I'm a little confused by this description. Is it saying that inside 'prototypes' for example has a dictionary inside it called 'val' which is a list? Or is it saying inside 'prototypes' there's a list called 'val'.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And, if there's only one thing inside 'prototypes', what's the logic for having another level?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The format of the dictionary is: {'DeployInt':{'prototypes':{'val':[]}, 'n_build':{'val':[]}, 'buildtimes':{'val':[]}, 'lifetimes':{'val':[]}} and this structure is because of the xml structure for the DeployInst:
<DeployInst> <prototypes> <val>str</val> <val>str</val> .... </prototypes> ... </DeployInst

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok! I was a little confused there, but that makes sense

scripts/create_AR_DeployInst.py Outdated Show resolved Hide resolved
scripts/create_AR_DeployInst.py Outdated Show resolved Hide resolved
scripts/create_AR_DeployInst.py Outdated Show resolved Hide resolved
scripts/create_AR_DeployInst.py Outdated Show resolved Hide resolved
scripts/tests/test_create_AR_DeployInst.py Outdated Show resolved Hide resolved
scripts/tests/test_create_AR_DeployInst.py Outdated Show resolved Hide resolved
scripts/tests/test_create_AR_DeployInst.py Outdated Show resolved Hide resolved
scripts/tests/test_create_AR_DeployInst.py Outdated Show resolved Hide resolved
scripts/tests/test_create_AR_DeployInst.py Outdated Show resolved Hide resolved
@nsryan2
Copy link
Member

nsryan2 commented Oct 20, 2022

Ok, everything looks good to me! I'll merge if it's all set

@nsryan2 nsryan2 merged commit 403359a into arfc:master Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants