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

[ENH] Tidy up test exclusions #473

Merged
merged 21 commits into from
Jul 10, 2023
Merged

[ENH] Tidy up test exclusions #473

merged 21 commits into from
Jul 10, 2023

Conversation

TonyBagnall
Copy link
Contributor

@TonyBagnall TonyBagnall commented Jun 5, 2023

This PR tidies up some of the exclusions in testing/_config.py and updates some of the issues

  1. Removes exclusions I dont think we need any more. Many referenced issues in the bad place which have since been closed. If they cause intermittent failure, I would like to see the fails, raise an issue and re-exclude again.
  2. Removes a redundant function from RISE
  3. Changes PlateauFinder to not store the intervals it uses
  4. Reduces TapNet testing size
  5. Reorders some of the classifier tests in anticipation of simplification (sorry)

I have reduced the scope of this to remove just the exclusion for PlateauFinder and HIVECOTEV2, I think maybe incermentally bring others back to give time for intermittent fault finding

Reference Issues/PRs

#472 #347 #271

@TonyBagnall TonyBagnall added the testing Testing related issue or pull request label Jun 5, 2023
@TonyBagnall TonyBagnall marked this pull request as ready for review June 6, 2023 07:52
@TonyBagnall TonyBagnall mentioned this pull request Jun 6, 2023
4 tasks
@TonyBagnall
Copy link
Contributor Author

@hadifawaz1999 I think this is just a random DL fail, I can see no reason for it failing, but could you take a look and see if there is anything
https://github.com/aeon-toolkit/aeon/actions/runs/5186055397/jobs/9346945374?pr=473

image

@hadifawaz1999
Copy link
Member

hadifawaz1999 commented Jun 8, 2023

@hadifawaz1999 I think this is just a random DL fail, I can see no reason for it failing, but could you take a look and see if there is anything
https://github.com/aeon-toolkit/aeon/actions/runs/5186055397/jobs/9346945374?pr=473

image

Hmm thats super weird it shouldn't do that, are you running it locally or just on CI ? will try the test multiple times locally today, will let you know what happens.

@TonyBagnall
Copy link
Contributor Author

This is from the CI, I think its a form of interference from saving on CI, but I can honestly not see how the network would be uninitialised

@TonyBagnall
Copy link
Contributor Author

TonyBagnall commented Jun 11, 2023

more limited un-exclusion:

  1. passes
  2. passes
  3. passes
  4. passes
  5. passes
  6. passes
  7. FAIL https://github.com/aeon-toolkit/aeon/actions/runs/5213205431/jobs/9506946636?pr=473
  8. passes
  9. passes
  10. passes

@hadifawaz1999
Copy link
Member

This is from the CI, I think its a form of interference from saving on CI, but I can honestly not see how the network would be uninitialised

The CI saving model is now fixed in #485

@MatthewMiddlehurst
Copy link
Member

MatthewMiddlehurst commented Jun 27, 2023

I don't think the latest failure (https://github.com/aeon-toolkit/aeon/actions/runs/5360306587/jobs/9734338503?pr=473) is on our end, so i would disregard it. Seems like its not only us codecov/codecov-action#1003.

The PR doesnt actually change the removed estimators list, so not sure why failure #7 happened! I have a hunch some of the DL ones are memory hungry and could be reduced, but thats just speculation

@TonyBagnall
Copy link
Contributor Author

now I have actually removed the exclusion, this has since ran four times without fail and is I think ready to go in

@hadifawaz1999
Copy link
Member

Do we have some clue about the other ones still excluded such as HC1 ? @TonyBagnall

@TonyBagnall
Copy link
Contributor Author

@hadifawaz1999 I'm waiting for @MatthewMiddlehurst 's big revamp of the tree ensembles before going there

Copy link
Member

@hadifawaz1999 hadifawaz1999 left a comment

Choose a reason for hiding this comment

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

LGTM!

@TonyBagnall TonyBagnall merged commit 338f98d into main Jul 10, 2023
@TonyBagnall TonyBagnall deleted the ajb/dl_tests branch July 10, 2023 14:59
@MatthewMiddlehurst MatthewMiddlehurst added the enhancement New feature, improvement request or other non-bug code enhancement label Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, improvement request or other non-bug code enhancement testing Testing related issue or pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants