Add default value for dump-dict-file

[pnunna93]

dump_dict_file is defined as optional argument, but the script raises error if its not defined. https://github.com/ROCmSoftwarePlatform/hipify_torch/blob/master/hipify_cli.py#L134

This PR adds a default value for dump_dict_file to avoid error when user doesn't define it.

Other option would be to make dump_dict_file a required argument.

CC: @AdrianAbeyta @jithunnair-amd

[pruthvistony]

This looks like a error which is introduced by some git merge/rebase. Please hold on to the merge on this change. When the dump-dict-file was added it was working properly for all scenarios, I will check more on it.

[jithunnair-amd]

@pruthvistony I believe your testing involved using the CMake file which calls hipify_cli.py with the dump-dict-file argument here: https://github.com/ROCmSoftwarePlatform/hipify_torch/blob/39af73f1b4d493b800ded70abdc2a48e81289363/cmake/Hipify.cmake#L85

@pnunna93 Are you invoking hipify_cli.py directly in your flow? Can you post a link?

[pnunna93]

@jithunnair-amd, yes I am invoking hipify_cli.py directly. Its not integrated into our flow yet, I tested it for small module in cupy through command line: python hipify_cli.py --config-json hipify_config.json

These are the contents of hipify_config.json: { "project_directory": "/myworkspace/cupy_new/cupy/cuda", "output_directory" : "/myworkspace/cupy_new/cupy/hip" }

[jithunnair-amd]

Yes, then @pruthvistony, I agree with @pnunna93 that either we should make it required https://github.com/ROCmSoftwarePlatform/hipify_torch/blob/master/hipify_cli.py#L57, or specify a default value like in this PR.

[jithunnair-amd]

@pnunna93 Use-case question for you: after invoking hipify_cli.py, do you plan to use the replace_cuda_with_hip_files.py script?

@pruthvistony I am thinking whether we should specify the same default value here: https://github.com/ROCmSoftwarePlatform/hipify_torch/blob/master/tools/replace_cuda_with_hip_files.py#L23, and then get rid of the explicit argument values and HIPIFY_DICT_FILE definition in Hipify.cmake, since I imagine most users don't care about the name of the hipify_dict_file, as it's an intermediate artifact for them at best? Not sure if we tried this already and ran into some technical issues.

[pnunna93]

We dont plan to use replace_cuda_with_hip_files.py script, we are keeping cuda and hip directories separately at the moment, so that it will be easier to upstream.

[jithunnair-amd]

We dont plan to use replace_cuda_with_hip_files.py script, we are keeping cuda and hip directories separately at the moment, so that it will be easier to upstream.

@pnunna93 Okay. Just as an fyi though: the reason we created that file was to ease developers' task of getting the list of equivalent hipified files given a list of CUDA files. So regardless of whether you are hipifying to a separate directory or not, this is something to consider if you're currently listing out the hipified files in your list of source files.

[pnunna93]

Sure @jithunnair-amd, good to know it helps with hip CUDA file mapping, will make use of it, thanks.

[pruthvistony]

--dump-dict-file argument was added by commit - https://github.com/ROCmSoftwarePlatform/hipify_torch/commit/98d6f08b8c6461ae4daf24d525e7141f7be9ceeb, it was handled correctly.

Later we had another commit - https://github.com/ROCmSoftwarePlatform/hipify_torch/commit/20826e4062526707b796ffa836874de3eefaaf96, which is a restructuring PR - broke it. :(

[pruthvistony]

@pnunna93 Use-case question for you: after invoking hipify_cli.py, do you plan to use the replace_cuda_with_hip_files.py script?

@pruthvistony I am thinking whether we should specify the same default value here: https://github.com/ROCmSoftwarePlatform/hipify_torch/blob/master/tools/replace_cuda_with_hip_files.py#L23, and then get rid of the explicit argument values and HIPIFY_DICT_FILE definition in Hipify.cmake, since I imagine most users don't care about the name of the hipify_dict_file, as it's an intermediate artifact for them at best? Not sure if we tried this already and ran into some technical issues.

This needs to be checked. After this PR, the line - https://github.com/ROCmSoftwarePlatform/hipify_torch/blob/23f53b025b466d8ec3c45d52290d3442f7fbe6b1/hipify_cli.py#L130 is obsolete. So want to have a relook at the whole logic in a separate activity.

[pruthvistony]

Added a story - https://github.com/ROCmSoftwarePlatform/frameworks-internal/issues/4108