deepskies / DeepCMBsim

MIT License
2 stars 0 forks source link

Update flow chart #71

Closed bnord closed 1 year ago

bnord commented 1 year ago

Current flowchart Created in google slides

bnord commented 1 year ago
  1. I think each module box should have both text and a symbol, and these terms should be directly related to what's in the code.
  2. I think we should consider this as part of what would go into the software documentation.
cavestruz commented 1 year ago
samueldmcdermott commented 1 year ago

thoughts on this new one? ex_workflow_alt.pdf

samueldmcdermott commented 1 year ago

(also: assigned this to @humnaawan )

bnord commented 1 year ago
screenshot_402
bnord commented 1 year ago
bnord commented 1 year ago

is renaming warranted?

?rename: params_io --> parameters_input_output? ?rename: config_obj --> configuration_object?

screenshot_404
bnord commented 1 year ago
bnord commented 1 year ago
bnord commented 1 year ago

Does the plotting module still exist? If not, we should still state that we use namaster if we keep that in there.

samueldmcdermott commented 1 year ago

is renaming warranted?

?rename: params_io --> parameters_input_output? ?rename: config_obj --> configuration_object?

screenshot_404

if that is a question, the answer is I don't think so (these names were suggested by others and I think they're clear), but if that is a direction then I am happy to take it. Please let me know if you do in fact have a preference for these changes

samueldmcdermott commented 1 year ago

Does the plotting module still exist? If not, we should still state that we use namaster if we keep that in there.

yes -- it is still there but it is optional (an "extra" in the parlance of the pyproject.toml). I was planning to specify in the caption or text that that is optional (hence the dashed line). It already states that it calls namaster -- are there other changes you would like on this, or is it okay?

samueldmcdermott commented 1 year ago

@bnord I edited most of your comments to include checklists, which I checked, but two of them required clarification, which I added in replies above. Let me know if my questions make sense, happy to go forward in any way with them.

New figure here (low-res for now but will improve that before putting into the paper) workflow-june2

samueldmcdermott commented 1 year ago

@bnord tagging you again

bnord commented 1 year ago

is renaming warranted? ?rename: params_io --> parameters_input_output? ?rename: config_obj --> configuration_object?

screenshot_404

if that is a question, the answer is I don't think so (these names were suggested by others and I think they're clear), but if that is a direction then I am happy to take it. Please let me know if you do in fact have a preference for these changes

If we remove matplotlib, should we remove the cl_plotting function from the flowchart? I'm thinking yes if I'm caught up on everything.

bnord commented 1 year ago

oh, sorry, I just saw that optional is a possibility. I would guess that it would be best practice for clarity to indicate the cl_plotting is optional in the figure.

bnord commented 1 year ago

I think I'd like to do some more tweaks to the figure, but it's probably inefficient (and maybe annoying for others) for me to make requests little-by-little. Do you want to re-assign this to me, and I can make a try?

samueldmcdermott commented 1 year ago

just assigned it to you @bnord -- feel free to make any edits you like in the google doc (or any way you like)

bnord commented 1 year ago

How does the latest version look?

https://docs.google.com/presentation/d/1-gDIkdLtom0yEvgFSj0C8N1egvZOjfOjp9rjqfLfI6U/edit#slide=id.g24eb1e1981e_0_60

bnord commented 1 year ago

here's my logic:

  1. solid blocks are for files, which could be scripts or a data file
  2. dashed blocks are for data elements

I know this doesn't adhere to the schools of thought for flowchart design, but I think it might be sufficiently self-consistent and human-readable.

samueldmcdermott commented 1 year ago

looks good to me! Only Q -- did you mean to say max_l twice in the lowest block? In CAMB there are both max_l and max_l_tensor so maybe the second one was supposed to add _tensor to it?

bnord commented 1 year ago

I updated it. Does that work?

https://docs.google.com/presentation/d/1-gDIkdLtom0yEvgFSj0C8N1egvZOjfOjp9rjqfLfI6U/edit#slide=id.g24eb1e1981e_0_60

samueldmcdermott commented 1 year ago

looks good to me

@humnaawan do you want to close this? If not done by, say, 2PM ET I'll close it