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

CMR #66

Merged
merged 1 commit into from
Sep 20, 2024
Merged

CMR #66

merged 1 commit into from
Sep 20, 2024

Conversation

paigerube14
Copy link
Collaborator

@paigerube14 paigerube14 commented Aug 20, 2024 •
edited
Loading

Type of change

  • Refactor
  • New feature
  • Bug fix
  • Optimization
  • Documentation Update

Description

This new algorithm compares a new run with any previous matching runs and generates a percent difference. This algorithm is for smaller amount of runs to analyze if there is a significant difference/regression than in the current run

If there are more than 1 previous runs to compare to the current run, the previous runs are meaned together to 1 to compare with the new run

NOTE: Further enhancements are needed to pass/fail based on a given tolerancy in the config.

Related Tickets & Documents

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.

Testing

https://mastern-jenkins-csb-openshift-qe.apps.ocp-c1.prod.psi.redhat.com/job/scale-ci/job/paige-e2e-multibranch/job/orion/36/console
percentdiff

@vishnuchalla vishnuchalla self-requested a review August 20, 2024 20:24
vishnuchalla
vishnuchalla reviewed Aug 22, 2024
View reviewed changes
pkg/algorithms/algorithmFactory.py
@@ -6,6 +6,7 @@
import pkg.constants as cnsts
from .edivisive import EDivisive
from .isolationforest import IsolationForestWeightedMean
from .cmr import CMR


class AlgorithmFactory: # pylint: disable= too-few-public-methods, too-many-arguments, line-too-long
Copy link
Collaborator

Choose a reason for hiding this comment

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

Our factory seems to be growing, can we have switch-case here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In prow we are using python3.9, from what I read online switch-case doesn't work with that python version

+ python --version
Python 3.9.9

https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_release/56698/rehearse-56698-periodic-ci-openshift-qe-ocp-qe-perfscale-ci-main-aws-4.18-nightly-x86-payload-control-plane-6nodes/1836716469891633152/artifacts/payload-control-plane-6nodes/openshift-qe-orion-cluster-density/build-log.txt

Copy link
Collaborator

@vishnuchalla vishnuchalla Sep 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Need not necessarily be a switch case. Even a dictionary to map those constants with functions should work so that we can have all the mappings at one place. I am okay if we want to take that in a different PR

vishnuchalla
vishnuchalla reviewed Aug 22, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
vishnuchalla
vishnuchalla reviewed Aug 26, 2024
View reviewed changes
pkg/utils.py Outdated Show resolved Hide resolved
shashank-boyapally
shashank-boyapally reviewed Aug 26, 2024
View reviewed changes
pkg/algorithms/cmr/cmr.py Outdated Show resolved Hide resolved
pkg/algorithms/cmr/cmr.py Outdated Show resolved Hide resolved
pkg/algorithms/cmr/cmr.py Outdated Show resolved Hide resolved
pkg/utils.py Outdated Show resolved Hide resolved
pkg/algorithms/cmr/cmr.py Outdated Show resolved Hide resolved
pkg/algorithms/cmr/cmr.py Outdated Show resolved Hide resolved
pkg/algorithms/cmr/cmr.py Outdated Show resolved Hide resolved
pkg/algorithms/cmr/cmr.py Outdated Show resolved Hide resolved
pkg/algorithms/cmr/cmr.py Outdated Show resolved Hide resolved
@paigerube14
Copy link
Collaborator Author

@shashank-boyapally @vishnuchalla @jtaleric any more comments here?

@shashank-boyapally
Copy link
Collaborator

Hi @paigerube14 lgtm from my side. just curious on the logic of max_time in the cmr algorithm.

@jtaleric
Copy link
Member

@paigerube14 I tried to access the console to see how to properly run --cmr

Running locally, i keep getting

(test) jtaleric at polaris in ~/code/orion on cmr_2* $ orion cmd --cmr --lookback-size 1
2024-09-20 15:44:51,297 - Orion      - INFO - file: orion.py - line: 124 - 🏹 Starting Orion in command-line mode
2024-09-20 15:44:51,301 - Orion      - INFO - file: utils.py - line: 218 - The test aws-416-med-scale-cluster-density-v2 has started
Traceback (most recent call last):
  File "/home/jtaleric/code/orion/test/bin/orion", line 8, in <module>
    sys.exit(cli())
             ^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/orion.py", line 126, in cmd_analysis
    output, regression_flag = run(**kwargs)
                              ^^^^^^^^^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/pkg/runTest.py", line 58, in run
    fingerprint_matched_df, metrics_config = process_test(
                                             ^^^^^^^^^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/pkg/utils.py", line 224, in process_test
    runs = match.get_uuid_by_metadata(metadata, fingerprint_index, lookback_date=start_timestamp, lookback_size=options['lookback_size'])
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: Matcher.get_uuid_by_metadata() got an unexpected keyword argument 'lookback_size'

@jtaleric
Copy link
Member

@paigerube14 I tried to access the console to see how to properly run --cmr

Running locally, i keep getting

(test) jtaleric at polaris in ~/code/orion on cmr_2* $ orion cmd --cmr --lookback-size 1
2024-09-20 15:44:51,297 - Orion      - INFO - file: orion.py - line: 124 - 🏹 Starting Orion in command-line mode
2024-09-20 15:44:51,301 - Orion      - INFO - file: utils.py - line: 218 - The test aws-416-med-scale-cluster-density-v2 has started
Traceback (most recent call last):
  File "/home/jtaleric/code/orion/test/bin/orion", line 8, in <module>
    sys.exit(cli())
             ^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/orion.py", line 126, in cmd_analysis
    output, regression_flag = run(**kwargs)
                              ^^^^^^^^^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/pkg/runTest.py", line 58, in run
    fingerprint_matched_df, metrics_config = process_test(
                                             ^^^^^^^^^^^^^
  File "/home/jtaleric/code/orion/test/lib64/python3.11/site-packages/pkg/utils.py", line 224, in process_test
    runs = match.get_uuid_by_metadata(metadata, fingerprint_index, lookback_date=start_timestamp, lookback_size=options['lookback_size'])
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: Matcher.get_uuid_by_metadata() got an unexpected keyword argument 'lookback_size'

Thanks to @shashank-boyapally I pulled in the latest fmatch and this error goes away.

@paigerube14
Copy link
Collaborator Author

@jtaleric awesome! The best way to do the lookback to see this feature work is a lookback of 3 (2 earliest will be meaned together and then compared to the latest one)

@paigerube14
Copy link
Collaborator Author

paigerube14 commented Sep 20, 2024 •
edited
Loading

Hi @paigerube14 lgtm from my side. just curious on the logic of max_time in the cmr algorithm.

@shashank-boyapally I feel like I had to set that to get the older results first. But will play around with taking that out and using the time from one of the runs instead

@jtaleric
Copy link
Member

/lgtm thanks for adding this @paigerube14 !

@paigerube14
geting percent difference
9476a1f 
rh-pre-commit.version: 2.2.0
rh-pre-commit.check-secrets: ENABLED

Signed-off-by: Auto User <[email protected]>
vishnuchalla
vishnuchalla approved these changes Sep 20, 2024
View reviewed changes
Copy link
Collaborator

@vishnuchalla vishnuchalla left a comment

Choose a reason for hiding this comment

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

lgtm

@paigerube14 paigerube14 merged commit 2c9f287 into cloud-bulldozer:main Sep 20, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shashank-boyapally shashank-boyapally shashank-boyapally left review comments

@vishnuchalla vishnuchalla vishnuchalla approved these changes

@jtaleric jtaleric Awaiting requested review from jtaleric

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

Successfully merging this pull request may close these issues.
4 participants
@paigerube14 @shashank-boyapally @jtaleric @vishnuchalla