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

add random behavior for size of sink request #550

Merged
merged 6 commits into from
Nov 28, 2023

Conversation

nuclearkatie
Copy link
Contributor

@nuclearkatie nuclearkatie commented Nov 21, 2023

  • Three new optional parameters to vary the size of the sink's request
  • random_size has default None, but also four two options that call the PRNG from the context:
    • "UniformReal", "NormalReal"
    • No longer implemented
      • "UniformInt", "NormalInt"
  • When using one of the normal distributions, two additional parameters random_size_mean and random_size_stddev are used as fractions of the maximum request size (calculated at each timestep RequestAmt())

Checklist

Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

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

If this is going into Cycamore, we might want to discuss what random sink behaviors make sense. I know you want a way to demo them all, but not sure any "Int" method makes sense?

src/sink.cc Outdated Show resolved Hide resolved
src/sink.cc Outdated Show resolved Hide resolved
src/sink.cc Outdated Show resolved Hide resolved
src/sink.cc Outdated Show resolved Hide resolved
src/sink.h Outdated Show resolved Hide resolved
@nuclearkatie
Copy link
Contributor Author

After deciding if fractional or fixed μ and σ makes more sense, my next question is how (if?) to restrict the user's choice of μ and σ. I expect few use cases would want to set the mean higher than the maximum allowable value (amt, which is the min of throughput and remaining inventory space), but do we need to actually restrict the user from setting that if that's their intent?

On the other hand, I worry that a lack of restrictions will result in users accidentally setting a distribution with a negligible probability of picking an allowable value-- we don't want to have to sample 100,000 times to get one value

@nuclearkatie
Copy link
Contributor Author

Cyclus version used for github actions needs to be updated with the PRNG (cyclus/cyclus#1599) for the build/test workflow to pass

@gonuke gonuke added this to the Random Behavior milestone Nov 25, 2023
@nuclearkatie nuclearkatie marked this pull request as ready for review November 27, 2023 14:46
@gonuke
Copy link
Member

gonuke commented Nov 27, 2023

Can you rebase this against main to update the testing? In the meantime we ( @bennibbelink ) need to sort out which images are available for testing downstream (cyclus/cyclus#1628)

@nuclearkatie
Copy link
Contributor Author

@gonuke this branch is already rebased against main, including the last update #549

@nuclearkatie nuclearkatie self-assigned this Nov 27, 2023
@gonuke
Copy link
Member

gonuke commented Nov 27, 2023

OK - I was thrown off by seeing failures of the CircleCI tests... but maybe those are lingering and not relevant.

@gonuke gonuke merged commit 4b477d1 into cyclus:main Nov 28, 2023
4 checks passed
nuclearkatie added a commit to nuclearkatie/cycamore that referenced this pull request Nov 28, 2023
@nuclearkatie nuclearkatie mentioned this pull request Nov 30, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Design unit test for randomness in request size for Sink archetype
2 participants