PR: Resource allocation plots in util.py#3382
PR: Resource allocation plots in util.py#3382charles-cowart merged 9 commits intoqiita-spots:devfrom
Conversation
antgonza
left a comment
There was a problem hiding this comment.
Looking good, a few comments/requests.
antgonza
left a comment
There was a problem hiding this comment.
Approved but it will be great if @charles-cowart can also do a pass. Thank you.
|
@Gossty it might be good to change the name of this PR if it's no longer a WIP and no longer just an initial commit. |
|
|
||
| ax = axs[0] | ||
| # models for memory | ||
| mem_model1 = (lambda x, k, a, b: k * np.log(x) + x * a + b) |
There was a problem hiding this comment.
It's not desirable to have these defined once in utils and once in tests. If you need them for tests, I would suggest moving them out of this function and defining them at the top of util.py. Then you can still use them in this function, but you can also import them into test_util.py (actually I think you just need to import the model_mem and model_time arrays) and use them there. In that way you're treating util.py as the place to go to define and get that information. Currently you have to know that this is defined in two places and make changes to both as needed.
charles-cowart
left a comment
There was a problem hiding this comment.
Looks good! Thanks for all your hard work!
No description provided.