feat: verbosity and random seed

[amoodie]

add verbosity and random seed options to the yaml parser

I changed the behavior of the verbose flag to be an integer rather than boolean.

• 0 is silent [default]
• 1 is some output on each timestep
• 2 is lots of output all over the place

You can now specify verbose: 1 in the yaml input to change behavior.

I changed the random seed setting to be a yaml option. You can now specify seed: 42 in the yaml input to set the random seed to 42. This happens only once, immediately following the yaml parsing. Default value of seed in yaml is -1, which does not alter the random seed at all, allowing for random runs. I didn't know a better place to put this, because set_constants gets called again and again if inlet boundary conditions are changed.

I made some other minor changes and fixes in docs etc.

[ericbarefoot]

Overall:

I like this addition. These are good things to be addressing for people interacting with the model.

Documentation

I appreciate all the stuff y'all are doing with the documentation. It seems like a lot of work, and I'll have to look into it sometime.

Verbosity

I'm super into this method of specifying some integer level of verbosity you're interested in. I can't remember right now, but I think I did a similar thing in my matlab refactoring, and it made a big difference.

random seed setting

I'm not sure about the logical flow for the random seed. My preference would be for the default to be None or something. Then that logic tree you have set up could just have one more step. maybe something like:

if self.seed is None:
    np.random.seed(np.random.uniform)
else:
    np.random.seed(self.seed)

[ericbarefoot]

I'm not sure about this. What if some goofball sets the seed to be a negative number? I could easily see someone doing that, in which case they'll get confusing behavior. Can we do something instead where the default is None and then this section of code does the logic parsing for that, specifying it to any random number if the person doesn't specify, and if they do specify, then use that one.

[amoodie]

@ericbarefoot yeah, I agree the logic here is bad. I think ideal would be seed: None in the yaml, and then

if self.seed:
    np.random.seed(self.seed)

It could be that simple, if you could make the default None... But, if you specify the type of seed in default.yml to be int, then seed: None in the config yaml, you get back a type error.

I couldn't readily think of a clean way to do it, and -1 is often used as a special case in python args, so I went with it. I'll look at this some more.

[amoodie]

err nvm that would evaluate to False for seed=0. Yours above would work though, I think, if None were int...

[amoodie]

@ericbarefoot yah I'm scratching my head over here for ways to do this.

I can get it working by specifying the default value as [null, 0] and then ignoring your yaml key type: and just using the type returned from the parsing of the yaml key default:, but this is really unreadable in the default file, and I think the default of -1 is much more intuitive. We can just specify in the docs that only positive integers are acceptable.

I don't want to do something like if type(user_dict[oo]) is expected_type or user_dict[oo] is None because that would open any key to be None too, which I think would be bad.

Happy to take suggestions on how to make it work with taking null or an integer.

[amoodie]

Also:

 import numpy as np

np.random.seed(-10)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "mtrand.pyx", line 244, in numpy.random.mtrand.RandomState.seed
  File "_mt19937.pyx", line 166, in numpy.random._mt19937.MT19937._legacy_seeding
  File "_mt19937.pyx", line 180, in numpy.random._mt19937.MT19937._legacy_seeding
ValueError: Seed must be between 0 and 2**32 - 1

So it wouldn't even work if user specified a negative value.

[amoodie]

something we could think about is a more flexible parser. This also resolves another issue, that we call eval() on arbitrary yaml; I think pyyaml does this already anyway, but avoiding calls to eval is always good.

We could implement something like numeric as a "type", that allows floats or ints, or lists [int, None] that allows either.

[ericbarefoot]

So it wouldn't even work if user specified a negative value.

That's good to know. I figured that the seed setting was a little more flexible. Since this is the case, I don't mind the -1 default actually. I was just worried about people throwing in random garbage accidentally. If numpy throws an error, then great.

but avoiding calls to eval is always good.

For sure. I couldn't think of a better way to test for type, and if there's a better way, then I'm happy to go that route.

[amoodie]

@ericbarefoot okay, I implemented something that increases the flexibility of the parser. You can now specify a list of acceptable options in the default list e.g., ['int', 'None'] and either will be acceptable for overriding the default value.

This allows the default value to be None (or really yaml'snull which becomes None) and anything else is set as the random seed. If nothing is specified, the seed is unmodified. If you pass a bad value e.g., seed: -10 you will get a numpy ValueError, as expected and described above.

Go ahead and merge if you approve 👍

[ericbarefoot]

I'm into this new way of doing the default yaml. LGTM