Cloud-CV / EvalAI

:cloud: :rocket: :bar_chart: :chart_with_upwards_trend: Evaluating state of the art in AI
https://eval.ai
Other
1.77k stars 788 forks source link

Fix #4274 Add or delete challenge phase/phase splits #4392

Open Harshit28j opened 4 months ago

Harshit28j commented 4 months ago

This PR resolves #4274

I debug each and every functionality and statements to find out the root cause of this problem and I figured out that issue is from the challenge_phase_split_uuids (which actually holds data value from yaml)

I removed the loop that tell us about the combination in which we have error and added a condition to append the combination if it does not exist in current_phase_split_id (which is from host)

This is how the workflow looks like for validate_challenge_phase_splits

image

Its somewhat similarly for validate_challenge_phase

Harshit28j commented 4 months ago

Here is the working:-

https://github.com/Cloud-CV/EvalAI/assets/48647625/c0784898-94e7-4b6f-925a-ad9de24241b6

codecov-commenter commented 4 months ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 69.27%. Comparing base (96968d6) to head (dcb9f0d). Report is 1110 commits behind head on master.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #4392 +/- ## ========================================== - Coverage 72.93% 69.27% -3.66% ========================================== Files 83 20 -63 Lines 5368 3574 -1794 ========================================== - Hits 3915 2476 -1439 + Misses 1453 1098 -355 ``` [see 64 files with indirect coverage changes](https://app.codecov.io/gh/Cloud-CV/EvalAI/pull/4392/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) [see 64 files with indirect coverage changes](https://app.codecov.io/gh/Cloud-CV/EvalAI/pull/4392/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/Cloud-CV/EvalAI/pull/4392?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/Cloud-CV/EvalAI/pull/4392?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Last update [be3c597...dcb9f0d](https://app.codecov.io/gh/Cloud-CV/EvalAI/pull/4392?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
gautamjajoo commented 4 months ago

@Harshit28j We should remove (1,2,2) instead of appending. Thoughts?

Harshit28j commented 4 months ago

@Harshit28j We should remove (1,2,2) instead of appending. Thoughts?

Yes, @gautamjajoo I agree. Also, since I removed the for loop that uses challenge_phase_split_uuids to respond to the user about missing UUIDs, I think there is no longer a need for the challenge_phase_split_uuids variable either.

Edit: I tested in my local code environment and it's working. Instead of removing missing combinations, I removed combinations that already exist, considering this case:

_Let's say we have new combinations from the yaml file (1,2,3) and current_challenge_phasesplit are from host (1,1,1), (1,2,1), (1,2,2). Then if a combination does not exist, it will show a ValueError.

So I only removed combinations that already exist, and it's working fine. I have tried and tested it by both adding and deleting.