Add new functions to support downloading from AWS s3 #218
Conversation
…into new_inference
…into new_inference
fang19911030
changed the title
|
@fang19911030 I got errors when invoking
Are any prerequisites required? |
|
after updated conda env and gempyor:
|
|
Hi @kjsato , I corrected the wrong function name used in unit tests. These errors are solved. |
I confirmed, thx |
…ename()' in utils.py
There was a problem hiding this comment.
Thank you, this is very good, I had some comments along the way we can discuss.
The end goal is to have a command line to resume like there is currently.gempyor-simulate and these functions should be adapted to be used through this entry point, that will read environment variable or command line argument through click.
| def create_resume_out_filename(filetype: str, liketype: str) -> str: | ||
| run_id = os.environ.get("FLEPI_RUN_INDEX") | ||
| prefix = f"{os.environ.get('FLEPI_PREFIX')}/{os.environ.get('FLEPI_RUN_INDEX')}" | ||
| inference_filepath_suffix = f"{liketype}/intermidate" |
| prefix = f"{os.environ.get('FLEPI_PREFIX')}/{os.environ.get('FLEPI_RUN_INDEX')}" | ||
| inference_filepath_suffix = f"{liketype}/intermidate" | ||
| inference_filename_prefix = "{:09d}.".format(int(os.environ.get("FLEPI_SLOT_INDEX"))) | ||
| index = "{:09d}.{:09d}".format(1, int(os.environ.get("FLEPI_BLOCK_INDEX")) - 1) |
There was a problem hiding this comment.
I think the create_file_name function handles the formatting of the index.
| ) | ||
|
|
||
|
|
||
| def get_parquet_types_for_resume() -> List[str]: |
There was a problem hiding this comment.
This is very good, but these functions should take arguments and not read environment variables, because we want to use them from gempyor (and have less and fewer environment variable), and because if we need an environment variable version we can add a wrapper around it.
(think of a script like simulate.py, that is e.g called resume_from.py : this script parse the environment variable from the safety of the click module and call these functions.
There was a problem hiding this comment.
Another note: seeding is a csv, so I think this function should be renamed "get_filetype_for_resume" (this 4 letter id is called filetype throughout the code)-
| return resume_file_name_mapping | ||
|
|
||
|
|
||
| def download_file_from_s3(name_map: Dict[str, str]) -> None: |
There was a problem hiding this comment.
Amazing this is very good. Ideally we also have a "move on filesystem" function that can be used as drop-in when resuming from another folder. In this case, it should just move the file in a safe way (creating folders along the way)
|
(again, sorry for the delay reviewing, as you know round 18 has been really hard) |
|
No worry, I will do change based the feedbacks |
Add new python functions which can be used to download files from AWS s3 to the utils.py