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

Add earth albedo effect for thermal calculation #688

Merged
merged 47 commits into from
Sep 27, 2024

Conversation

kai0722
Copy link
Member

@kai0722 kai0722 commented Sep 12, 2024

Related issues

#687

Description

The albedo calculation is now performed in the same way as for the calculation of solar heat input.

Test results

スクリーンショット 2024-09-20 17 22 42 When not under eclipse, Albedo is reflected in the order of one-tenth of the solar heat input.

Impact

The behavior of thermal analysis feature changes.

Supplementary information

@kai0722 kai0722 self-assigned this Sep 12, 2024
@kai0722 kai0722 requested review from sksat and a team as code owners September 12, 2024 10:02
@kai0722 kai0722 requested review from 200km, ogoogo and Hiro-0110 and removed request for a team September 12, 2024 10:02
@kai0722 kai0722 linked an issue Sep 12, 2024 that may be closed by this pull request
@kai0722
Copy link
Member Author

kai0722 commented Sep 12, 2024

アルベド計算をおこない,太陽光入熱とともに軌道上熱入力計算に反映するようにしました.
基本的にはすでに実装されていた太陽光入熱関数を模擬する形で実装を進めましたが,太陽位置と地球位置を内部で取得するために,引数を
local_celestial_information->GetPositionFromSpacecraft_b_m(c_sun)
から
const LocalCelestialInformation* local_celestial_information
に変えています.その関係で若干まどろっこしくなっているところ(temperature.cpp 126〜143あたり)があります.

@conjikidow
Copy link
Member

@kai0722 ありがとうございます。
検証結果の

executed correctly

について,より詳細に数字やグラフなどが載せられたら載せてもらえると嬉しいです。

@kai0722
Copy link
Member Author

kai0722 commented Sep 12, 2024

ログファイルでいいですかね?

@conjikidow
Copy link
Member

なるべくこちらがぱっと見て理解できるようにしてもらえると助かります 🙏

@200km 200km added priority::medium priority medium dynamics dynamics calculation major update incompatible API changes labels Sep 13, 2024
Copy link
Member

@200km 200km left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修正案ありがとうございます。
コメントつけました。
また、コンフリクト解除もお願いします。

src/dynamics/thermal/node.hpp Outdated Show resolved Hide resolved
src/dynamics/thermal/node.hpp Outdated Show resolved Hide resolved
src/dynamics/thermal/node.cpp Outdated Show resolved Hide resolved
src/dynamics/thermal/heatload.hpp Outdated Show resolved Hide resolved
src/dynamics/thermal/heatload.hpp Outdated Show resolved Hide resolved
src/dynamics/thermal/temperature.hpp Outdated Show resolved Hide resolved
src/dynamics/thermal/temperature.hpp Outdated Show resolved Hide resolved
src/dynamics/thermal/temperature.cpp Outdated Show resolved Hide resolved
src/dynamics/thermal/temperature.cpp Outdated Show resolved Hide resolved
src/dynamics/thermal/temperature.cpp Outdated Show resolved Hide resolved
@kai0722
Copy link
Member Author

kai0722 commented Sep 20, 2024

アルベド計算式で,衛星が地球の影に入っているときはアルベド0になることを考慮できていなかったので,追加する.
はじめにS/Cベクトルと太陽方向ベクトルの内積を考えることで影or not を判定するようにする.

@200km
Copy link
Member

200km commented Sep 20, 2024

はじめにS/Cベクトルと太陽方向ベクトルの内積を考えることで影or not を判定するようにする

影判定はすでにLocaEnvironmentでやっているので、その機能を使ってください。

@200km
Copy link
Member

200km commented Sep 21, 2024

@kai0722 アルベドのiniファイルはその修正分もpushしてください。(ただし、計算のデフォルト値はDISABLEにしてください)

@kai0722
Copy link
Member Author

kai0722 commented Sep 21, 2024

@kai0722 アルベドのiniファイルはその修正分もpushしてください。(ただし、計算のデフォルト値はDISABLEにしてください)

User側でいじっていたため失念していました.反映しました.

Copy link
Member

@200km 200km left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修正ありがとうございます。議論も収束してきたのでこの指摘の修正が終わったらapproveできると思います。

data/sample/initialize_files/sample_local_environment.ini Outdated Show resolved Hide resolved
src/environment/local/local_environment.hpp Outdated Show resolved Hide resolved
src/environment/local/earth_albedo.hpp Outdated Show resolved Hide resolved
src/environment/local/earth_albedo.hpp Outdated Show resolved Hide resolved
src/environment/local/earth_albedo.hpp Outdated Show resolved Hide resolved
src/environment/local/earth_albedo.cpp Outdated Show resolved Hide resolved
src/environment/local/earth_albedo.cpp Show resolved Hide resolved
src/dynamics/thermal/temperature.hpp Show resolved Hide resolved
Copy link
Member

@200km 200km left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コメントに対する小さな質問一つだけお願いします。CIが動いていないので、私の手元で動かして問題なければApproveします。

src/dynamics/thermal/node.cpp Outdated Show resolved Hide resolved
@200km 200km changed the title Feature/upgrade thermal analysis Add earth albedo effect for thermal calculation Sep 26, 2024
@200km 200km added the environment environment calculation label Sep 26, 2024
Copy link
Member

@200km 200km left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI治ったのでそのビルドの中でwarningが出ていたので修正をお願いします。

D:\a\s2e-core\s2e-core\src\dynamics\thermal\node.cpp(46,10): warning C4189: 'earth_distance_m': local variable is initialized but not referenced [D:\a\s2e-core\s2e-core\src\dynamics\DYNAMICS.vcxproj]

@kai0722
Copy link
Member Author

kai0722 commented Sep 26, 2024

CI治ったのでそのビルドの中でwarningが出ていたので修正をお願いします。

D:\a\s2e-core\s2e-core\src\dynamics\thermal\node.cpp(46,10): warning C4189: 'earth_distance_m': local variable is initialized but not referenced [D:\a\s2e-core\s2e-core\src\dynamics\DYNAMICS.vcxproj]

修正しました

Copy link
Member

@200km 200km left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修正ありがとうございます。Approveしたのでマージお願いします。

@kai0722 kai0722 merged commit a95ff06 into develop Sep 27, 2024
11 checks passed
@kai0722 kai0722 deleted the feature/upgrade-thermal-analysis branch September 27, 2024 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dynamics dynamics calculation environment environment calculation major update incompatible API changes priority::medium priority medium
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Upgrade thermal analysis calc to include Albedo flux
3 participants