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

fix(interpolation): fix spline bug #1541

Merged
merged 2 commits into from
Nov 14, 2024
Merged

Conversation

yhisaki
Copy link

@yhisaki yhisaki commented Sep 18, 2024

Description

This PR fixes a spline interpolation bug, details are described here.

Related links

How was this PR tested?

https://evaluation.ci.tier4.jp/evaluation/reports/c5a304ec-f26a-5b86-b63e-c675203d6669?project_id=autoware_dev

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

Signed-off-by: Y.Hisaki <[email protected]>
Copy link

sonarcloud bot commented Sep 18, 2024

@yhisaki yhisaki marked this pull request as ready for review September 18, 2024 02:03
@1222-takeshi 1222-takeshi changed the base branch from beta/v0.3.16.1 to beta/v0.3.19 November 6, 2024 06:30
Copy link

sonarcloud bot commented Nov 11, 2024

@1222-takeshi
Copy link

@YoheiMishina @yn-mrse @peeeechi @sfukuta

(原理的にはspline補間のバグを修正したのみなので,CPU負荷に変化は無いはずです)

久木さんから上記のコメントいただいており、Evaluatorでのシナリオ評価によるデグレチェック、シミュレータベースでのCPU, memoryリソースのデグレチェックを実施し、本PRにおける特筆すべきデグレがないことを確認したので、PRをマージしたいと思います。
皆さんから見て気になる点等ございましたら、コメントいただけると幸いです。

解析に詳細についてはそれぞれ以下にまとめてあります。
Evaluatorでのシナリオ別デグレチェックの詳細
クライテリアあり環境におけるデグレチェックの詳細
リソース周りにおけるデグレチェックの詳細

@sfukuta
Copy link

sfukuta commented Nov 12, 2024

@1222-takeshi
Evaluatorでのシナリオ別デグレチェックの詳細
こちらは、変更前は毎回OKだったものが、修正後は毎回NGになっていたりしないですか?
ランダムfailはわかるのですが、その観点でのコメントは欲しいです。

@sfukuta
Copy link

sfukuta commented Nov 12, 2024

@1222-takeshi
クライテリアあり環境におけるデグレチェックの詳細
3の結論が要確認になっているので、確認した結果を更新して欲しいです。

@sfukuta
Copy link

sfukuta commented Nov 12, 2024

@1222-takeshi
リソース周りにおけるデグレチェックの詳細
結論がよくわからないとあるので、三浦さんがチェックして問題ないなら、その旨、修正して欲しいです。

@1222-takeshi
Copy link

@sfukuta
コメントありがとうございます!
それぞれチケットのEvidenceに下記のコメント追加しました。

こちらは、変更前は毎回OKだったものが、修正後は毎回NGになっていたりしないですか?
ランダムfailはわかるのですが、その観点でのコメントは欲しいです。

修正の前も後も常にfailしているようなシナリオは存在しない → これもランダムfailを疑う理由

3の結論が要確認になっているので、確認した結果を更新して欲しいです。

停留所間での変化量と停留所発着時の変化量を見ればいいので問題なし

結論がよくわからないとあるので、三浦さんがチェックして問題ないなら、その旨、修正して欲しいです。

CPU, Memoryともにデータから読み取れるような特筆すべき差分はない (システム全体に影響するような差分はない) ため問題なしと判断

@sfukuta
Copy link

sfukuta commented Nov 13, 2024

@1222-takeshi
変更点について問題ないことを確認しました。

@sfukuta
Copy link

sfukuta commented Nov 13, 2024

@1222-takeshi
Evaluatorでのシナリオ別デグレチェックの詳細

ランダムFailは問題ありませんが、3回のテストを実施した時に、少なくとも全てのテストが1回はOKになっているという認識です。その認識通りであれば、全てテストが1度は、PASSしている事の記載をお願いします。

@sfukuta
Copy link

sfukuta commented Nov 13, 2024

リソース周りにおけるデグレチェックの詳細

リソースをチェックした元のevaluatorは、Evaluatorでのシナリオ別デグレチェックの詳細 で実施した結果をもとに、検証している理解であっていますか?
認識が合っている場合は、それが分かる記載をevidenceにお願いします。

@1222-takeshi
Copy link

@sfukuta

3回のテストを実施した時に、少なくとも全てのテストが1回はOKになっているという認識です。その認識通りであれば、全てテストが1度は、PASSしている事の記載をお願いします。

Yesで、Evidenceに追記しました。

リソースをチェックした元のevaluatorは、Evaluatorでのシナリオ別デグレチェックの詳細 で実施した結果をもとに、検証している理解であっていますか?
認識が合っている場合は、それが分かる記載をevidenceにお願いします。

こちらもYesです。
参考にしたjobは元々Evidenceには書いてあったので、前のチケットの結果を受けている旨を追記してわかりやすくしておきました!

@sfukuta
Copy link

sfukuta commented Nov 14, 2024

@1222-takeshi マージして大丈夫です。

@1222-takeshi 1222-takeshi merged commit c99b4d8 into beta/v0.3.19 Nov 14, 2024
9 of 10 checks passed
@1222-takeshi 1222-takeshi deleted the tmp/hisaki-eve branch November 14, 2024 06:08
@1222-takeshi 1222-takeshi restored the tmp/hisaki-eve branch November 15, 2024 05:37
1222-takeshi added a commit that referenced this pull request Nov 15, 2024
@1222-takeshi
Copy link

マージ先を間違えていたので、一旦revertして以下のPRを作り直しました。
#1649

1222-takeshi added a commit that referenced this pull request Nov 15, 2024
Revert "fix(interpolation): fix spline bug (#1541)"

This reverts commit c99b4d8.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants