Correct modin samples by amyskov · Pull Request #768 · oneapi-src/oneAPI-samples

amyskov · 2021-11-24T09:23:45Z

Existing Sample Changes

Description

This PR corrects Modin related samples. Changes are next:

Change config setting technique for more appropriate for python file.
Specify config settings for newer Modin versions (since 0.12.0 version config for OmniSci enabling was changed).
Remove compression parameter from read_csv function (csv file compression is inferred automatically and explicit notion of compression parameter causes "default-to-pandas" behavior and performance degradation).
Remove nrows parameter from read_csv function to avoid "default-to-pandas" behavior (nrows parameter is not supported yet in Modin with OmniSci backend).
DataFrame.query function calls replaced with equivalents to avoid "default-to-pandas" behavior and avoid performance degradation (DataFrame.query is not supported yet in Modin with OmniSci backend).
Alternative installation guidance was fixed, sample.json file was set similarly to MODIN: fix getting started samples testing in CI #863 change.
Warnings were disabled to avoid unneeded output cluttering.
Fixed minor typos.

External Dependencies

List any external dependencies created as a result of this change.

Type of change

Please delete options that are not relevant. Add a 'X' to the one that is applicable.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Implement fixes for ONSAM Jiras

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

amyskov · 2021-11-24T14:01:42Z

Hi @JoeOster ! Could you take a look on this and on the #769 PRs?

praveenkk123 · 2021-11-24T17:37:23Z

Please submit to development branch but not the master branch

amyskov · 2021-11-25T10:22:46Z

Please submit to development branch but not the master branch

@praveenkk123 target branch was changed.

praveenkk123

Approve for CI

raoberman

@amyskov can you also ensure that the conda install commands are correct for the README of the sample? conda install modin should be good i believe .

https://github.com/oneapi-src/oneAPI-samples/blob/master/AI-and-Analytics/Getting-Started-Samples/IntelModin_GettingStarted/README.md

raoberman · 2021-11-30T23:55:24Z

@louie-tsai @praveenkk123 please approve for 2022.1 since the change for census is important (after README edit i requested as conda install command currently listed is not latest)

amyskov · 2021-12-01T13:18:38Z

@amyskov can you also ensure that the conda install commands are correct for the README of the sample? conda install modin should be good i believe .

https://github.com/oneapi-src/oneAPI-samples/blob/master/AI-and-Analytics/Getting-Started-Samples/IntelModin_GettingStarted/README.md

@raoberman verified conda install command was added.

amyskov · 2021-12-06T11:29:03Z

Additional changes:

nrows parameter was removed from read_csv function to avoid "default-to-pandas" behavior (nrows parameter is not supported yet in Modin with OmniSci backend).
Warnings were disabled to avoid unneeded output cluttering.

raoberman · 2022-01-18T21:27:00Z

@amyskov please re-submit this PR to the master branch. See Praveen and I's email for info

The base branch was changed.

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

amyskov · 2022-01-19T12:51:13Z

@amyskov please re-submit this PR to the master branch. See Praveen and I's email for info

@raoberman PR was re-summited and rebased.

amyskov · 2022-01-19T12:53:57Z

Additionally Modin configuration (engine setting) now depends on it's version (since 0.12.0 version config for OmniSci enabling was changed).

praveenkk123

Approve for CI

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

amyskov · 2022-01-21T15:49:42Z

Modin configurations were changed to avoid triggering modin-project/modin#4017 issue. Also installation instructions were slightly modified to speed-up packages installation.

amyskov · 2022-01-24T12:47:46Z

Hi @praveenkk123!
For some reason Samples validation (Linux) test failed with strange output, while if i run steps commands from sample.json file tests passes (as i understood this file contains CI settings). Can you please provide me with instructions to reproduce CI fail? Or maybe you can give me some suggestions how i can tune CI to pass tests?

dchigarev · 2022-02-17T15:46:13Z

@amyskov please rebase on the current master, we recently merged #863 that should fix CI testing

praveenkk123

Approve for CI

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

amyskov · 2022-02-18T10:06:04Z

@praveenkk123 Census sample CI config was changed as well, but jupyter nbconvert was used instead of runipy because of hardcoded timeout value in runipy (see jupyter/jupyter_client#197 for details), that raised error during long IO step execution. Can you, please, restart the CI?

praveenkk123

Approve for CI

praveenkk123

Approve for CI

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

amyskov · 2022-03-03T09:40:20Z

@praveenkk123 merge conflict was fixed, could you please restart the CI?

praveenkk123

Approve for CI

praveenkk123 · 2022-03-03T19:08:05Z

All tests passed. Can I merge this?

amyskov · 2022-03-04T07:46:27Z

All tests passed. Can I merge this?

Yes, sure.

amyskov changed the base branch from master to development November 25, 2021 09:52

praveenkk123 previously approved these changes Nov 29, 2021

View reviewed changes

raoberman suggested changes Nov 30, 2021

View reviewed changes

praveenkk123 added 2022.1 2022.2 and removed 2022.1 labels Dec 1, 2021

mdbtucker force-pushed the development branch 2 times, most recently from f414747 to bf3bcbc Compare December 10, 2021 11:12

amyskov changed the base branch from development to master January 19, 2022 11:16

amyskov added 6 commits January 19, 2022 14:43

correct Census sample

450f980

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

correct getting started sample

415d66b

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

correct conda install command

0507ba0

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

correct sample.json, modify conda install cmd

08b8d2d

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

fix typo

73d608b

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

disable warnings

30537f6

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

amyskov force-pushed the correct_modin_samples branch 3 times, most recently from 068b416 to 8833db6 Compare January 19, 2022 12:28

specify configuration setting for newer modin versions

68ff984

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

amyskov force-pushed the correct_modin_samples branch from 8833db6 to 68ff984 Compare January 19, 2022 12:45

praveenkk123 previously approved these changes Jan 20, 2022

View reviewed changes

amyskov added 2 commits January 21, 2022 15:58

correct configuration setting

5e80316

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

correct instalation instructions

f3bb7ae

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

amyskov dismissed praveenkk123’s stale review via f3bb7ae January 21, 2022 14:59

praveenkk123 self-requested a review January 21, 2022 18:42

praveenkk123 previously approved these changes Jan 21, 2022

View reviewed changes

Merge branch 'master' into correct_modin_samples

4f8e100

JoeOster dismissed praveenkk123’s stale review via 4f8e100 February 17, 2022 16:10

praveenkk123 previously approved these changes Feb 17, 2022

View reviewed changes

correct CI for modin sample

6682d8d

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

amyskov dismissed praveenkk123’s stale review via 6682d8d February 18, 2022 09:59

praveenkk123 approved these changes Feb 18, 2022

View reviewed changes

praveenkk123 previously approved these changes Mar 2, 2022

View reviewed changes

apply recent changes

0eb96f9

Signed-off-by: Alexander Myskov <alexander.myskov@intel.com>

amyskov dismissed praveenkk123’s stale review via 0eb96f9 March 3, 2022 09:38

praveenkk123 approved these changes Mar 3, 2022

View reviewed changes

praveenkk123 merged commit 8844f27 into oneapi-src:master Mar 4, 2022

Conversation

amyskov commented Nov 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Existing Sample Changes

Description

External Dependencies

Type of change

How Has This Been Tested?

Uh oh!

amyskov commented Nov 24, 2021

Uh oh!

praveenkk123 commented Nov 24, 2021

Uh oh!

amyskov commented Nov 25, 2021

Uh oh!

praveenkk123 left a comment

Choose a reason for hiding this comment

Uh oh!

raoberman left a comment

Choose a reason for hiding this comment

Uh oh!

raoberman commented Nov 30, 2021

Uh oh!

amyskov commented Dec 1, 2021

Uh oh!

amyskov commented Dec 6, 2021

Uh oh!

raoberman commented Jan 18, 2022

Uh oh!

amyskov commented Jan 19, 2022

Uh oh!

amyskov commented Jan 19, 2022

Uh oh!

praveenkk123 left a comment

Choose a reason for hiding this comment

Uh oh!

amyskov commented Jan 21, 2022

Uh oh!

amyskov commented Jan 24, 2022

Uh oh!

dchigarev commented Feb 17, 2022

Uh oh!

praveenkk123 left a comment

Choose a reason for hiding this comment

Uh oh!

amyskov commented Feb 18, 2022

Uh oh!

praveenkk123 left a comment

Choose a reason for hiding this comment

Uh oh!

praveenkk123 left a comment

Choose a reason for hiding this comment

Uh oh!

amyskov commented Mar 3, 2022

Uh oh!

praveenkk123 left a comment

Choose a reason for hiding this comment

Uh oh!

praveenkk123 commented Mar 3, 2022

Uh oh!

amyskov commented Mar 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

amyskov commented Nov 24, 2021 •

edited

Loading