search

llm-jp / scripts

Apache License 2.0
1 stars 1 forks source link

Add setup script for mdx #35

Closed YumaTsuta closed 2 months ago

YumaTsuta commented 2 months ago

This PR add script to set up the MDX environment by mounting /data and /model directories.

YumaTsuta commented 2 months ago

@hkiyomaru 特に理由はないのですが、口頭で話していたのでレビュお願いできるでしょうか？（他に良さそうな人がいればその人でも）

hkiyomaru commented 2 months ago

ディレクトリに簡単な README.md があると良いと思います．また，参照できてナンボのスクリプトなので，llm-jp/documents （加えて取り急ぎ mdx 環境構築の Google docs）にこのスクリプトへのリンクを追加してください．

hkiyomaru commented 2 months ago

動作確認できました．めちゃくちゃ便利ですね．

hkiyomaru commented 2 months ago

すみません，ファイル名をよく見ていませんでした．単に Lustre のセットアップを行うスクリプトなので，setup_mdx.sh より setup_lustre.sh の方が適切な気がします．

YumaTsuta commented 2 months ago

@hkiyomaru コメント対応しました。 README.mdも必要なので、フォルダを setup_lustreに変更、スクリプトは setup.shにしました。

YumaTsuta commented 2 months ago

no_arg_setup.sh は ip -br addrで出力される２つ目のエントリ（３行目）が常に STORAGE_NETWORK1_IPV4のエントリなのではないか、という推定の元で作成しています。ある程度使用した上で問題なさそうなら将来的に置き換えることを見据えて作成してみたのですがダメでしょうか？

hkiyomaru commented 2 months ago

動作保証のないスクリプトを main ブランチに突っ込むのは得策ではないと思います．この PR では間違いなく動作する setup.sh にフォーカスすることにして，no_arg_setup.sh は setup.sh をより便利にするための機能追加の PR として切り出してはどうでしょうか？

YumaTsuta commented 2 months ago

なるほど。了解です。削除しました