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

Comments #2

Open
angelhof opened this issue Dec 6, 2023 · 1 comment
Open

Comments #2

angelhof opened this issue Dec 6, 2023 · 1 comment

Comments

@angelhof
Copy link
Member

angelhof commented Dec 6, 2023

I am trying to run max-temp from scratch and I will gather all the potentially confusing things that I find here so that we can fix them :) I am writing them as they come so the writing is not very clear, let me know if something doesn't make sense

  • Before "running benchmarks" section, we need to say a few things about the directory structure, e.g., "Each directory contains a different benchmark suite, more details about each benchmark suite cna be found in the separate READMEs in the separate directories"
  • Why do we need to source the script whole_shebang and can't we just execute it, e.g., ./whole_shebang.sh. We might need to chmod +x ./whole_shebang.sh first.
  • The name whole shebang does not give a lot of info about what this is supposed to do haha, can we change it to something like, run_bench/run_suite/etc?
  • --flags should not be mentioned in the main readme, it should be possible to call the main run script of its directory without any flags and it should do something reasonable.
  • Let's call each directory a suite and not a type of benchmark
  • Installation: It really is downloading and not installation and I think it should just be git clone IMO (instead of wget).
  • For each individual suite we know roughly how long it takes to set up inputs so we can print it as a message in whole-shebang
  • ./max-temp source whole_shebang.sh did not work. I managed to run the suite by doing cd max-temp; source whole_shebang.sh.
  • max-temp:
    • No need to show "Start"
    • "Setting up inputs" should be removed because it is misleading
    • There is no message that shows that we are running the experiment (and how much is it expected to take roughly)
    • Why is run.sh in inputs? I would expect all scripts to be at the top level of the suite (or in a scripts folder).
    • NOAA and atlas indexes are in different format! (see the sed in new-temp-analytics.sh)
    • data and outputs should be added to a gitignore file inside the max-temp directory since they are temporary directories.
  • If we interrupt the process while it runs it stays in the internal directory. Maybe we need to use ( cd inputs; ./inputs.sh ) instead of cding?
@angelhof
Copy link
Member Author

angelhof commented Dec 6, 2023

@dileventi you can take a look at those whenever you have time :)

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

No branches or pull requests

1 participant