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

Create Flow Chart #50

Closed
bnord opened this issue Jun 13, 2023 · 25 comments
Closed

Create Flow Chart #50

bnord opened this issue Jun 13, 2023 · 25 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@bnord
Copy link
Contributor

bnord commented Jun 13, 2023

Create a flow chart of the code to explain to us how the package works and for publishing.

@bnord bnord added the enhancement New feature or request label Jun 13, 2023
@bnord bnord added this to the alpha release milestone Jun 13, 2023
@bnord bnord self-assigned this Jun 13, 2023
@bnord
Copy link
Contributor Author

bnord commented Jun 13, 2023

Here's a draft: I based this on the DeepCMBSim flow.

@evavagiakis What do you think? How should I update?

@evavagiakis
Copy link
Collaborator

  • Flowchart currently lacks the option to load in an existing halo (M,z) catalog to run Generate_Clusters.py (like in slide 3)
  • The names of those functions as I’m working with them are currently different (get_dm_halo.py and make_sz_cluster.py), we can call them anything of course, but I think halo and cluster might sound too similar
  • Right now I have the visualizations (plotting submaps using observational inputs) as a separate python script called visualization.py; do we still want this broken out, or as a part of one of those scripts in the flow chart?

@bnord
Copy link
Contributor Author

bnord commented Jun 13, 2023

great comments!

I will work on the others, but one thing is perhaps immediately important is that @voetberg and @samueldmcdermott have shown or made it clear that including visualization scripts (especially that rely on matplotlib) may bloat the package unnecessarily.

Following the lead of DeepCMBSim, we may want to not exclude visualizations as a required part of the package.

@evavagiakis
Copy link
Collaborator

That makes sense. To clarify, should we plan on excluding visualizations as part of the package?

@bnord
Copy link
Contributor Author

bnord commented Jun 14, 2023

I think, for the most part, yes. But I also think @samueldmcdermott and @voetberg have a way through DeepCMBSim to include them in the notebooks. I think the idea is to just not require matplotlib as a package for the simulator installation.

@bnord
Copy link
Contributor Author

bnord commented Jun 14, 2023

Should GenerateHalos.py subsume the concept of reading in an existing halo set. Or should we make that a separate option and box for for the flow?

@bnord
Copy link
Contributor Author

bnord commented Jun 15, 2023

@evavagiakis:

  1. is the new naming scheme better?
  2. how essential is the injection of a halo data file for the flow chart? I think this flowchart will go into the paper.
  3. what do you think is important visualizations? Should we aim to ship the simulator with visualization functions?

@bnord
Copy link
Contributor Author

bnord commented Jun 19, 2023

@deepskies/team-sz-cluster-2023 check out the flow chart here so you can get a sense of the overall code structure.

The Battaglia profile business goes into Make_SZ_Cluster

@evavagiakis
Copy link
Collaborator

  1. I think the new naming scheme is better
  2. I think a typical user would be interested in defining their own z, M distribution, and one of the ways they would likely want to define that is by uploading their own distribution file. I am leaning towards making this an option in the alpha release but if it adds too much complexity it isn't needed.
  3. We can just skip visualizations I think.

@bnord
Copy link
Contributor Author

bnord commented Jun 19, 2023

I'm interested in trying to make custom halo distributions possible.

Here's what I think we'd need to do to make that happen (not in any particular order yet)

  1. define a data structure that the user needs to abide by
  2. define a file type/format that the user needs to be abide by -- e.g., h5 or csv

Questions:

  1. Would the user provide actual haloes, that we would then sample?
  2. Would the user provide specific haloes?

@bnord
Copy link
Contributor Author

bnord commented Jun 19, 2023

Depending on the conclusion we come to here, I can create a new issue to specify the user-defined halo inputs.

@evavagiakis
Copy link
Collaborator

Could you clarify the distinction between actual vs. specific halos?

@bnord
Copy link
Contributor Author

bnord commented Jun 23, 2023

ah yeah, that was unclear.

When you say "custom," which of the following do you mean?

  1. Would the user provide a distribution from which would we sample? This is currently planned, as we mimic other repos to provide distribution parameters.
  2. Would the user provide an actual list of haloes with masses and redshifts?

@evavagiakis
Copy link
Collaborator

When I say "custom" I mean 2., since I am assuming the default/basic input would be for the user to define a distribution (for example, the flat one I had been recently working with, with a zmin/zmax, Mmin/Mmax). "Custom" might be for the user to, for example, have a list of real luminous red galaxy z, M parameters that they have. In which case this is an existing dataset of z=[0.564,0.124,....0.895] M=[etc.] that the user wishes to simulate to replicate a real dataset. I don't think this is required to start but might be nice to include.

@bnord
Copy link
Contributor Author

bnord commented Jun 23, 2023

Coolio. I started a new issue for this #74 and put it on our wishlist.

@bnord
Copy link
Contributor Author

bnord commented Jun 23, 2023

In the meantime, I updated the flow chart to reflect our intention to import
https://docs.google.com/presentation/d/1iAJLM7Jcg6IymmK01ZZyCDdiiJy3U4FxsCEF2I_i7dY/edit#slide=id.g251f35f8f2c_0_78

@bnord
Copy link
Contributor Author

bnord commented Jun 24, 2023

I wonder if it's worth to include boxes within generate cluster that represent the steps going from pressure profile to sz decrement. I think that could help clarify for the user how simple but extensible our method is.

@evavagiakis
Copy link
Collaborator

I think including boxes in each of the .py files in the spreadsheet would make sense to me. I am not sure if "halo" and "cluster" in the inputs boxes might be confusing/redundant

@bnord
Copy link
Contributor Author

bnord commented Jun 26, 2023

What do you think is a good way to handle the "halo"-"cluster" dichotomy.

We often refer to the "SZ cluster" as if the cluster is painted onto the halo.

@evavagiakis
Copy link
Collaborator

In the inputs, I'm not sure we are inputting any information about the baryon distribution at this time, just halo parameters z, M. The functions in generate_cluster then define the cluster. So maybe we just remove "cluster parameters" from the user_config.yaml box unless they are referring to something else?

@bnord
Copy link
Contributor Author

bnord commented Jun 26, 2023

@evavagiakis
Copy link
Collaborator

@elaine-ran will continue to work on this for JOSS paper

@bnord
Copy link
Contributor Author

bnord commented Sep 1, 2023

edit at will!

@evavagiakis
Copy link
Collaborator

Moving paper-related tasks to discussion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants