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.

Sign up for GitHub

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

Jump to bottom

TEST: remove outdated tests from deselected_test.yaml #2046

Merged
merged 1 commit into from
Nov 6, 2024
Merged

TEST: remove outdated tests from deselected_test.yaml #2046

merged 1 commit into from
Nov 6, 2024

Conversation

JuliaRS
Copy link
Contributor

@JuliaRS JuliaRS commented Sep 12, 2024 •
edited
Loading

Description

Add a comprehensive description of proposed changes

List issue number(s) if exist(s): #6 (for example)

Checklist to comply with before moving PR from draft:

PR completeness and readability

  • I have reviewed my changes thoroughly before submitting this pull request.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation to reflect the changes or created a separate PR with update and provided its number in the description, if necessary.
  • Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details).
  • I have added a respective label(s) to PR if I have a permission for that.
  • I have resolved any merge conflicts that might occur with the base branch.

Testing

  • The unit tests pass successfully.
  • I have run it locally and tested the changes extensively.

Performance

  • I have measured performance for affected algorithms using scikit-learn_bench and provided at least summary table with measured data, if performance change is expected.
  • I have provided justification why performance has changed or why changes are not expected.

@JuliaRS
Copy link
Contributor Author

JuliaRS commented Sep 12, 2024

/intelci: run

@JuliaRS
Copy link
Contributor Author

JuliaRS commented Sep 12, 2024

/intelci: run

1 similar comment
@JuliaRS
Copy link
Contributor Author

JuliaRS commented Sep 12, 2024

/intelci: run

@samir-nasibli
Copy link
Contributor

CI job: http://intel-ci.intel.com/ef710169-ae96-f1a4-a50a-a4bf010d0e2e

@samir-nasibli
Copy link
Contributor

@JuliaRS I think this is still in progress. Moving into draft

@samir-nasibli samir-nasibli marked this pull request as draft September 15, 2024 06:52
@JuliaRS
Copy link
Contributor Author

JuliaRS commented Sep 19, 2024

/intelci: run

@samir-nasibli
Copy link
Contributor

Please try to keep your branch updated, rebase it with main

@JuliaRS JuliaRS marked this pull request as ready for review September 25, 2024 10:19
@JuliaRS
Copy link
Contributor Author

JuliaRS commented Sep 25, 2024

/intelci: run

2 similar comments
@JuliaRS
Copy link
Contributor Author

JuliaRS commented Sep 25, 2024

/intelci: run

@JuliaRS
Copy link
Contributor Author

JuliaRS commented Sep 25, 2024

/intelci: run

@JuliaRS JuliaRS force-pushed the deselected_tests branch 2 times, most recently from 08555ee to 37d4973 Compare September 25, 2024 12:59
@JuliaRS
Copy link
Contributor Author

JuliaRS commented Sep 25, 2024

/intelci:run

@JuliaRS
Copy link
Contributor Author

JuliaRS commented Sep 25, 2024

/intelci: run

@samir-nasibli
Copy link
Contributor

samir-nasibli commented Sep 25, 2024 •
edited
Loading

@JuliaRS I would suggest just reverse use deselected_tests as a test cases to check. Try this experiment.
This will reduce the job time. Successfully passed tests could be removed from deselect list

@samir-nasibli samir-nasibli changed the title remove outdated tests from deselected_test.yaml [experiment]: remove outdated tests from deselected_test.yaml Sep 25, 2024
@samir-nasibli samir-nasibli marked this pull request as draft September 25, 2024 13:13
@JuliaRS
Copy link
Contributor Author

JuliaRS commented Oct 16, 2024

/intelci: run

@JuliaRS JuliaRS force-pushed the deselected_tests branch 2 times, most recently from f6e306d to 22f9fa6 Compare October 16, 2024 16:41
@JuliaRS
Copy link
Contributor Author

JuliaRS commented Oct 16, 2024

/intelci: run

@JuliaRS
Copy link
Contributor Author

JuliaRS commented Oct 18, 2024

/intelci: run

@JuliaRS
Copy link
Contributor Author

JuliaRS commented Oct 21, 2024

/intelci: run

@JuliaRS JuliaRS marked this pull request as ready for review October 28, 2024 08:45
icfaust
icfaust reviewed Oct 28, 2024
View reviewed changes
Copy link
Contributor

@icfaust icfaust left a comment

Choose a reason for hiding this comment

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

Looking good, I think some discussion of reduced_tests needs to occur before merge.

deselected_tests.yaml Outdated
reduced_tests:
- cluster/tests/test_affinity_propagation.py
- cluster/tests/test_bicluster.py
Copy link
Contributor

Choose a reason for hiding this comment

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

@ethanglaser I don't know what exactly reduced_tests is used for (or if anything calls run_xpu_tests with 'reduced'). Will this section impact overall runtime (i.e. should we readd these to reduce CI times)? I'm leaning towards these staying in deselected_tests, but I need another opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@icfaust yes, I don't understand the difference between reduce_tests section and the ordinary deselected_tests section.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like we are running with reduced flag in internal CI. @homksei do you know what the reduced section is intended for?

Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly, I do not have information about the reasons why this was done.

@JuliaRS
Copy link
Contributor Author

JuliaRS commented Nov 4, 2024

/intelci: run

@JuliaRS JuliaRS requested a review from icfaust November 4, 2024 13:45
ethanglaser
ethanglaser reviewed Nov 4, 2024
View reviewed changes
deselected_tests.yaml Outdated Show resolved Hide resolved
@ethanglaser
Copy link
Contributor

This is nearly ready for merge, nice work! Looks like a recent PR was merged with an update to deselected tests so please rebase and run intelci once more. I have also started two nightly jobs just in case there are some sporadic issues that haven't shown up yet:

http://intel-ci.intel.com/ef9ac180-c207-f1e5-89e5-a4bf010d0e2d
http://intel-ci.intel.com/ef9ac1ca-46d4-f1f7-9170-a4bf010d0e2d

@JuliaRS
Copy link
Contributor Author

JuliaRS commented Nov 4, 2024

/intelci: run

@JuliaRS
Copy link
Contributor Author

JuliaRS commented Nov 4, 2024

/intelci: run

@JuliaRS
Copy link
Contributor Author

JuliaRS commented Nov 5, 2024

/intelci:run

@Alexsandruss
Copy link
Contributor

/intelci: run

@samir-nasibli samir-nasibli changed the title [experiment]: remove outdated tests from deselected_test.yaml TEST: remove outdated tests from deselected_test.yaml Nov 5, 2024
samir-nasibli
samir-nasibli approved these changes Nov 5, 2024
View reviewed changes
Copy link
Contributor

@samir-nasibli samir-nasibli left a comment

Choose a reason for hiding this comment

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

Good to go! Thank you @JuliaRS

@samir-nasibli samir-nasibli merged commit 1e1d536 into uxlfoundation:main Nov 6, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ethanglaser ethanglaser ethanglaser approved these changes

@Alexsandruss Alexsandruss Alexsandruss approved these changes

@samir-nasibli samir-nasibli samir-nasibli approved these changes

@icfaust icfaust icfaust approved these changes

@ahuber21 ahuber21 Awaiting requested review from ahuber21

@napetrov napetrov Awaiting requested review from napetrov

@homksei homksei Awaiting requested review from homksei

@syakov-intel syakov-intel Awaiting requested review from syakov-intel

@david-cortes-intel david-cortes-intel Awaiting requested review from david-cortes-intel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@JuliaRS @samir-nasibli @ethanglaser @Alexsandruss @icfaust @homksei