s2e-core icon indicating copy to clipboard operation
s2e-core copied to clipboard

Create data/external dir & move GeoPotential files

Open sksat opened this issue 1 year ago • 10 comments

Related issues

  • #611

Description

ExtLibraries/GeoPotential is not library. Just a data. So, it should be move to data dir.

  • Create data/external dir
  • Move ExtLibraries/GeoPotential to data/external/geo_potential
  • Merge scripts/Common/download_EGM96coefficients.sh into data/external/geo_potential
  • Add EXT_DAYA_DIR_FROM_EXE virtual environment value for config files
    • So, S2E read data/external files directory
    • No longer need to "build" and "install" step

Test results

Provide the test results and a link to the detailed results log.

Impact

Users should be migrate ExtLibraries/GeoPotential path reference to EXT_DIR_FROM_EXE/geo_potential

sksat avatar Feb 15 '24 00:02 sksat

修正提案ありがとうございます。

今の状況ですが、

  • ExtLibraries/cspiceの中にもde430.bspなど、天体に関するデータは含まれている
  • ExtLibraries/nrlmsise00の中にもSpaceWeather-v1.2.txtなどの天体に関するデータは含まれている
  • ExtLibraries/LunarGravityFieldも同様に存在する
  • dataの中に外部から落としてきたデータを入れる場合は、initialize_files/gnssのような場所に入れようとしている

という感じです。なので、Geopotentialだけ動かすのはそれはそれで整合性が取れなくなるかなと思います。

今の分類の方針としては、

  • ExtLibrariesには、
    • 外部からソースダウンロードしているライブラリに関連したデータを保存している
      • cspice, nrlmsise00
    • ほとんど更新が必要にならない(ユーザー毎にファイルが変わらない)データを保存している
      • cspice, geopotential, lunar gravity
      • nrlmsise00は更新しなくてもよい使い方も考えられるが、場合によっては1日毎に変更できるファイルもあるので、少し微妙
  • data/initialize_fileには、
    • 必須ではないし、シミュレーション条件によって頻繁に変わるデータが保存される(data内はユーザー毎に自由度高く編集される想定)
      • gnss、thermal、コンポーネントの詳細データなど
    • しかも、iniファイル内でどのファイルを参照するかを決めている

という感じになっていると思います。

方針を変更して整理する場合、次のような特性を考慮して分け方を考えた方が良いのかなと思います。

  • 必須データかどうか
    • cspiceは必須
    • 他はiniの設定によってなくても動きはする
  • ユーザー毎に変わるものか、高頻度に変わるものか
    • cspice, geopotential, lunar gravityはそんなに高頻度で変わらない
    • gnssはユーザーのシミュレーション対象時期によって高頻度に変わる
    • nrlmsise00は両者の中間で、高頻度で変わらない平均値的なファイルもあり得るし、1日毎に変わるファイルもあり得る

この特性を踏まえて、dataディレクトリ配下に置くべきなのかなどを議論するのが良いかなと思います。

また、ここに出てきておらず、みてみぬふりをしているものとして、src/library/external/igrf/igrf11.coefなどもあります。次のような特性を持っていますが、これを機に別に移動させても良い気もします。

  • これは性質としてはgeopotentialとかに近い
    • 5年に一回更新されるので、geopotentialなどよりは高頻度
  • 参考にしているigrfコードに紐づいているので、伝統的にこの場所に保管されている

200km avatar Feb 15 '24 00:02 200km

はい.これはその通りで,あくまで最初のミニマルな改善提案だというのと,単にレビューコストを下げるためにパッチを分割していただけです > Geopotentialだけ動かすのはそれはそれで整合性が取れなくなる

必須データかどうか,によって分け方を考えた方がよい,というのはあんまりわかっていないです. これはディレクトリの分割方針というよりかは,ドキュメントやスクリプトの責務な気がしています. 例えば,「標準的なファイルだけまとめてダウンロードする」みたいなスクリプトはあってよいと思いますが,ディレクトリとは別の話かなと.

データのライフサイクルという観点だと,今重要なのは「ビルド前に(操作が)必要か(ビルド時に必要なライブラリのダウンロード)」「ビルド時に必要か(リンクする)」「実行時に必要か(初期化時に読み込む)」という区別が重要だと考えていて,この PR は実行時に必要なモノをすべて data に移動したい,というアイデアです.

sksat avatar Feb 15 '24 01:02 sksat

あー,なるほど.S2E user ごとに管理するべきものと,各シミュレーションケース(ex: data/sample)毎に管理すべきデータがあるのではないか,という話か.理解しました. > 頻度

sksat avatar Feb 15 '24 01:02 sksat

ということであれば,data ディレクトリに置くべきかどうかの議論,というよりは,data ディレクトリのどこに置くべきか,という議論をするのが建設的な気がしますね.そうなるとここでも external はちょっと一般的すぎて微妙な命名かもしれません.

sksat avatar Feb 15 '24 01:02 sksat

とりあえず Issue 立てました: #613

sksat avatar Feb 15 '24 01:02 sksat

データ関連ファイルは次のような分類になると思います。

  • どのs2e-userでも共通のもの
    • geopotential, lunar gravity field, igrf.coef, de430.bspなど滅多に変わらないもの
  • s2e-user毎に変わるもの
    • gnss, 宇宙天気, thermal, コンポの特性CSVファイルなど、衛星が変わると変わるもの
    • satellite.ini、structure.iniなどはこちらに入るかもしれない
  • ユーザー一人一人がその時々に変える可能性があるもの
    • シミュレーション開始時間、log取得頻度、ノイズ・外乱の大きさなどいわゆるiniファイル

これまで、s2e-coreをsubmoduleとして取り込まない場合も想定しており、ExtLibrariesは複数のs2e-userで共有される可能性があったので、どのs2e-userでも共通のもの = ExtLibrariesとなっていました。 今はs2e-user内でsubmoduleでs2e-coreを取り込み、それぞれでExtLibrariesを生成することを推奨しているので、どのs2e-userでも共通のものという分け方にこだわる必要もないのかもしれません

data ディレクトリのどこに置くべきか,という議論をするのが建設的な気がしますね.

そうですね。基本的にはそれに賛成です。 ただ、唯一気になっているのはExtLibraries/cspicecspice/de430.bspなどが同じcspiceでまとまっている方がわかりやすいかもしれないという点です。de430.bspなどは中身を見てもよくわからないspice専用データで、これがExtLibraries/cspiceと近くに配置されてることはある程度見やすさの観点でありかなと思ったりもします。この辺りは広く意見を聞きたいです。

これを気にせずにdataに集約するなら、次のような分類分けが今の構成を自然に拡張できるかなと思っています。

- data
  - logs
  - initialize_files
    - simulation_base
    - environment
      - gnss.ini
      - gravity_field
        - geopotential, lunar gravity field
      - magnetic_field
        - igrf.coef
      - space_weather
        - SpaceWheather.txt
      - cspice
        - de430.bspなど
      - gnss
        - sp3ファイルなど
    - hoge_satellite
      - satellite.ini
      - structure.ini
      - local_environment.ini
      - disturbance.ini
      - thermal_csv_files
      - components
        - いろんなiniファイル
        - コンポーネントによってはCSVファイル
    - fuga_satellite
    - hoge_ground_station
      - ground_station.ini
      - components
        - antenna.ini
        - antenna_pattern.csv

200km avatar Feb 15 '24 15:02 200km

@sksat こちら、上の提案でよければ私の方で全て整理しますがいかがでしょうか?

200km avatar Feb 23 '24 06:02 200km

なるほど,これは特に考えていなかったです & s2e-core を取り込む想定でよいと思います > s2e-coreをsubmoduleとして取り込まない場合も想定

これには明確に反対の立場です.出自とカテゴリが似通っているというだけで,ビルド時に使うモノとランタイムに使うモノはデータのライフサイクルが異なるからです.これらのデータの場所を固定することによって ini ファイル内で s2e-core のディレクトリ構造や実行時のパスからの s2e-core へのパスなどを考慮しないといけなくなってしまうのはユーザ体験が非常に悪いと感じています. > ExtLibraries/cspicecspice/de430.bspなどが同じcspiceでまとまっている方がわかりやすいかもしれない.

initialize_files ディレクトリは不要な気がしますが,基本的にはそのような分類を行うことに賛成です. > 次のような分類分け

sksat avatar Mar 11 '24 02:03 sksat

「初期化のためのファイル」みたいな分類をディレクトリ単位でする必要はそんなにないのでは,とは思っています.入力・出力にそれぞれたくさんのファイル・ディレクトリがあるのであればまだしも,出力ファイルは logs に出ることが明らかですし.

sksat avatar Mar 11 '24 02:03 sksat

「初期化のためのファイル」みたいな分類をディレクトリ単位でする必要はそんなにない

逆にinitialize_filesディレクトリがあるデメリットや、なくなった時のメリットなどを教えてください。

ちなみに私としては、シミュレーション設定のためのファイルが一つのディレクトリにまとまっている方が各種説明を行う時に楽ですし、設定のコピペなどもまとまっている方が楽など、ディレクトリ分けした方がメリットがあると思っています。

200km avatar Mar 12 '24 16:03 200km

@sksat @suzuki-toshihir0 こちら、だいぶ時間が経ってしまいましたが、どうしましょうか。ここまでで、次のどちらが良いかというのに議論は収束されたかなと思います。initialize_filesというディレクトリを一度挟むかどうかという話です。

- data
  - logs
  - initialize_files
    - simulation_base
    - environment
      - gnss.ini
      - gravity_field
        - geopotential, lunar gravity field
      - magnetic_field
        - igrf.coef
      - space_weather
        - SpaceWheather.txt
      - cspice
        - de430.bspなど
      - gnss
        - sp3ファイルなど
    - hoge_satellite
      - satellite.ini
      - structure.ini
      - local_environment.ini
      - disturbance.ini
      - thermal_csv_files
      - components
        - いろんなiniファイル
        - コンポーネントによってはCSVファイル
    - fuga_satellite
    - hoge_ground_station
      - ground_station.ini
      - components
        - antenna.ini
        - antenna_pattern.csv
- data
  - logs
  - simulation_base
  - environment
    - gnss.ini
    - gravity_field
      - geopotential, lunar gravity field
    - magnetic_field
      - igrf.coef
    - space_weather
      - SpaceWheather.txt
    - cspice
      - de430.bspなど
    - gnss
      - sp3ファイルなど
  - hoge_satellite
    - satellite.ini
    - structure.ini
    - local_environment.ini
    - disturbance.ini
    - thermal_csv_files
    - components
      - いろんなiniファイル
      - コンポーネントによってはCSVファイル
  - fuga_satellite
  - hoge_ground_station
    - ground_station.ini
    - components
      - antenna.ini
      - antenna_pattern.csv

200km avatar May 28 '24 13:05 200km

AOCSミーティングで出た別の案は、dataディレクトリをなくして階層を浅くしたら良いのではというもの。srcなどと並列して、logsinitialize_filesが並ぶ形

- logs
- initialize_files
  - simulation_base
  - environment
    - gnss.ini
    - gravity_field
      - geopotential, lunar gravity field
    - magnetic_field
      - igrf.coef
    - space_weather
      - SpaceWheather.txt
    - cspice
      - de430.bspなど
    - gnss
      - sp3ファイルなど
  - hoge_satellite
    - satellite.ini
    - structure.ini
    - local_environment.ini
    - disturbance.ini
    - thermal_csv_files
    - components
      - いろんなiniファイル
      - コンポーネントによってはCSVファイル
  - fuga_satellite
    - 上と同様
  - hoge_ground_station
    - ground_station.ini
    - components
      - antenna.ini
      - antenna_pattern.csv

200km avatar Jul 05 '24 02:07 200km

  • 「シミュレーション設定のためのファイルが一つのディレクトリにまとまっている方が各種説明を行う時に楽」に関しては、むしろ僕は data/initialize_files にあったり ExtLibraries にあったり(さらにモノによっては src にあったり)して、非常に苦心していました............
  • コピペに関しては、個人的には initialize_files ごとコピペしたいことは新しい S2E user を用意した時ぐらいしかないと思っています
    • その時でも結局なんのためにどのファイルが必要なのかをちゃんと整理したくなると、各トピック毎とか、各衛星毎とかにコピペして調整する、というのがよくあるフローだと思います
  • (若干 off-topic かもですが)ディレクトリにファイルが入っているのは自明なので _files という suffix は余計ではないでしょうか?

などの観点で initialize_files というディレクトリは不要なのではないか、というのがこちらの意見です(整理)。

sksat avatar Jul 11 '24 07:07 sksat

data ディレクトリをなくす、というのはどのような観点での案なんでしょうか?まあ data もあまり強い意味の無い命名なので無くてもいいかも、ということであればそれはそれで分かります。ただ、logsinitialize_files が並列している対象は src ではなく、(S2E の)executable だと思います。例えば、コードは変えないけどたくさんシミュレーションを走らせるためだけのリポジトリがあったとして、その構造が

  • logs
  • initialize_files のようになるのは理解できます。

sksat avatar Jul 11 '24 08:07 sksat

コメントありがとうございます。

data/initialize_files にあったり ExtLibraries にあったり(さらにモノによっては src にあったり)して、非常に苦心していました............

これを解決するのが今回の提案なので、今回の提案以降では今までより明確にシミュレーション設定のためのファイルが一つのディレクトリにまとまっている方が各種説明を行う時に楽になると思っています。

新しい S2E user を用意した時ぐらいしかないと思っています

複数人でパラメータ(特定のセンサのノイズの大きさ、初期姿勢など)を色々振ってゲイン調整していくとき、少し条件を変えたものを共有するとき、昔試した条件を再現するため(logディレクトリに保存されたinitialize_filesをそのままコピーして再現する)などでコピーすることはたくさんあるかなと思います。

ディレクトリにファイルが入っているのは自明なので _files という suffix は余計ではないでしょうか?

initializeが動詞で、他のディレクトリと同じように名詞にするためにfilesがついているのだと理解しています。fileを外すなら、initializerなど名詞にするのが良いのかなと思っています。

data ディレクトリをなくす、というのはどのような観点での案なんでしょうか?

ディレクトリを深くしたくないからというのが理由です。私自身はあまり賛成はしていません。

200km avatar Jul 11 '24 09:07 200km

ディレクトリを深くしたくないの補足

data
- logs
- initialize_files
  - いろんなinitializeファイル

data
- logs
- いろんなinitializeファイル

と比べて嫌な点は、initialize_filesが一つあるせいで階層が深くなって、読みづらい ということだったので、それならそもそもdataがなければ一階層減らせるよという感じです。

200km avatar Jul 11 '24 09:07 200km

僕は data は特に無くてよくて

.
├── src
├── initialize_files(名前はさておき、)
├── logs

みたいな感じでよいのではないかな、という思想寄りです。ここで敢えて data にファイルを押し込めて階層を深くする必要性をあまり感じていない、というくらいです。

suzuki-toshihir0 avatar Jul 11 '24 10:07 suzuki-toshihir0

なので、個人的には単に「浅い階層にいた方が嬉しくない?」というくらいのモチベーションなので、それでs2e userを構成したときに構造上の問題が起きる、などであればそちらを解決する方向に従うつもりです。

suzuki-toshihir0 avatar Jul 11 '24 10:07 suzuki-toshihir0

浅い階層にいた方が嬉しい、というのは完全に同意です。その理由として src と並列、というロジックは違和感があったので理由を確認していました。

sksat avatar Jul 11 '24 10:07 sksat

あー。そういうことか。src と並列にしたい、ではなく、あくまで浅い階層に引っ張り上げたい、です。

suzuki-toshihir0 avatar Jul 11 '24 10:07 suzuki-toshihir0

なるほどそれはたしかに通常のユースケースとしてもありますね。名前と現状(結局それ以外にもファイルが散らばっている)はともかくとして、initialize_files 相当のディレクトリを切ること自体は妥当そうです。 > 昔試した条件を再現するため(logディレクトリに保存されたinitialize_filesをそのままコピーして再現する)

sksat avatar Jul 11 '24 10:07 sksat

自分もこちらと同意見です。

.
├── src
├── initialize_files(名前は別途要議論)
├── logs

意図としては

.
├── src
├── data
    ├── logs
    ├── 様々な初期化ファイル群

だと input と output が同列でごちゃまぜになるのが嫌なので

.
├── src
├── data
    ├── initialize_files(名前は別途要議論)
    ├── logs

が良く,ただそれならわざわざ data として input と output をまとめて階層を深くする必要もないかな,という次第です。

conjikidow avatar Jul 11 '24 10:07 conjikidow

完全に納得 > input と output が同列でごちゃまぜになるのが嫌

sksat avatar Jul 11 '24 10:07 sksat

ちなみに initialize_files の命名については自分も微妙だと思っています。 「初期化」というよりもシミュレーションの実行時の設定という感覚なので,settings や,

ただ、logs と initialize_files が並列している対象は src ではなく、(S2E の)executable だと思います。

を意識するなら simulation_config などでしょうか。

conjikidow avatar Jul 11 '24 11:07 conjikidow

ですよね。ここにあるのは設定ないし初期データであって、「初期化」はここを読んで(S2E)が行う動作でしかない。 > 「初期化」というよりもシミュレーションの実行時の設定という感覚

sksat avatar Jul 11 '24 11:07 sksat

議論ありがとうございます。議論の流れから、当初提案されていた「initialize_files的なディレクトリを削除する」という方向性はなくなったと思います。私もinitialize_files的なディレクトリは必要と主張してきたので、賛成です。それは確定でお願いします。

dataディレクトリが必要かどうか

上の流れから、@sksatさんが階層を浅くするため、dataを消してsrcとlogsなどを並列にしたいのか、srcとlogsなどが並列なのは意味的に違和感なので、dataを維持して階層も今のままにするのが良いのかどちらかいまいちわかりませんでした。
結局どちらでしょう? 個人的には階層の浅さは特に重要でないので、srcとlogsなどが並列なのは意味的に違和感なので、dataを維持して階層も今のままにするが良いですが、強い意見ではないのでどちらでも良いとは思ってもいます。

initialize_files の命名

元々は、.iniファイルのことを指していましたが、上の提案の修正でそれ以外のファイルも入るのでこのタイミングで名前を変更するのはありだと思います。 simulation_configは略さないという命名のルール的には、simulation_configurationですね。ただ、このファイル名と被るのが嫌だなと思います。

settingsが個人的にはいいかなと思います。

200km avatar Jul 11 '24 13:07 200km

@200km 遅くなってしまってすみません。ディレクトリ構成は私は以前お伝えしたとおり

.
├── src
├── initialize_files(名前は別途要議論)
├── logs

派です(強いこだわりというほどではないですが、決めの問題だと思います)。

名前については settings でよいと思います。 (CC: @sksat )

suzuki-toshihir0 avatar Jul 16 '24 10:07 suzuki-toshihir0

data を消す案のモチベーションが最初分からなかったので確認していただけで、理由に納得した + 階層を浅くしたいので、僕も

  • src
  • logs
  • settings

の構成でよいと思います。 simulation_configuration は長すぎる上に別の概念と被るので、名前は settings でよいと思います。

sksat avatar Jul 16 '24 10:07 sksat

ご返信ありがとうございます。では、次のような感じで修正しようと思います。

- src
- logs
- settings
  - simulation_base
  - environment
    - gnss.ini
    - gravity_field
      - geopotential, lunar gravity field
    - magnetic_field
      - igrf.coef
    - space_weather
      - SpaceWheather.txt
    - cspice
      - de430.bspなど
    - gnss
      - sp3ファイルなど
  - hoge_satellite
    - satellite.ini
    - structure.ini
    - local_environment.ini
    - disturbance.ini
    - thermal_csv_files
    - components
      - いろんなiniファイル
      - コンポーネントによってはCSVファイル
  - fuga_satellite
    - 上と同様
  - hoge_ground_station
    - ground_station.ini
    - components
      - antenna.ini
      - antenna_pattern.csv

200km avatar Jul 16 '24 10:07 200km

#613 に引き継いで作業も始めているので、これはCLOSEします。

200km avatar Jul 20 '24 09:07 200km