FEAT-#5367: Introduce new API for repartitioning Modin objects

[anmyachev]

Signed-off-by: Myachev [email protected]

What do these changes do?

• first commit message and PR title follow format outlined here

  NOTE: If you edit the PR title to match this format, you need to add another commit (even if it's empty) or amend your last commit for the CI job that checks the PR title to pick up the new PR title.

• passes flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
• passes black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
• signed commit with git commit -s
• Resolves FEAT: create API for repartitioning Modin objects #5367
• tests added and passing
• module layout described at docs/development/architecture.rst is up-to-date

[anmyachev]

@dchigarev could you take a look?

[dchigarev]

Overall looks good!

The only thing that I would propose to change is to move the function to modin.distributes.pandas module, it seems that the module is more suitable for partition-like stuff.

[anmyachev]

mypy error: error: Call to untyped function "__constructor__" in typed context [no-untyped-call]

[anmyachev]

@dchigarev ready for review

[dchigarev]

Looks good to me.

Although, want to hear more opinions from @modin-project/modin-core about this new API

[dchigarev]

can we add a section with some examples (what is bad partitioning, when one should use this function, what speed-up can re-partitioning bring) probably with a reference to according page in modin docs

[anmyachev]

In order to be in time for the next new release, I would prefer to do it separately.

Users sometimes ask why the performance is bad, first we can ask the user how many internal partitions and offer to use this feature if necessary. By improving the performance guide, they will be able to do it without us.

[YarShev]

A couple of comments from my side.

1. Is there an example where we would benefit with this new API?
2. from_partitions and unwrap_partitions were originaly added for those developing ML stuff to be able to retrieve underlying partitions to pass those in to ML parts. I am not sure that the new repartition API is nice to those who use pandas API and want to move from it to Modin. That would make it difficult to move from pandas to Modin.
3. I suppose repartitioning should happen transperantly to users.

[anmyachev]

A couple of comments from my side.

1. Is there an example where we would benefit with this new API?
2. from_partitions and unwrap_partitions were originaly added for those developing ML stuff to be able to retrieve underlying partitions to pass those in to ML parts. I am not sure that the new repartition API is nice to those who use pandas API and want to move from it to Modin. That would make it difficult to move from pandas to Modin.
3. I suppose repartitioning should happen transperantly to users.

1. This should help in case of MultiIndex takes up a huge amount of storage space #5247.
2. Are you suggesting moving it to another location? Any suggestion where?
3. We don't have implicit repartitioning along the column axis yet. Also, if we implement repartitioning in the same way as for the row axis, I'm not sure if this will help in the case described in MultiIndex takes up a huge amount of storage space #5247.

I guess I can make this function as a private method for dataframe and series, just like _to_pandas.

[dchigarev]

@YarShev

1. Is there an example where we would benefit with this new API?

I would say that the introduced API is some kind of a temporary hack for a user's code while we're dealing with an actual issue that caused problematic partitioning in their code.

P.S. another concrete case where this API helps is #5296 (compare time measurements with "default" and "proper" partitioning).

3. I suppose repartitioning should happen transperantly to users.

Sure, this API doesn't imply that users have to master partitioning mechanism and play it on their own to fix their performance. As I said earlier, it's supposed to be a one-line hack that improves performance significantly right now, not weeks later when we merge an actual fix for the partitioning problem that a user has faced.

[YarShev]

Okay, I am rather for <df/s>._repartition() as a temporary solution for the mentioned problems. Can you also create an issue for partitioning problem, which would happen transperantly to users in necessary cases.

@modin-project/modin-core, other thoughts?

[anmyachev]

@dchigarev @YarShev ready for review

[anmyachev]

@YarShev ready for review

[vnlitvinov]

We have PandasDataframePartitionManager.rebalance_partitions() which is much more advanced, I wonder why aren't we using it here and what is the difference in this new API and that function?

[anmyachev]

We have PandasDataframePartitionManager.rebalance_partitions() which is much more advanced, I wonder why aren't we using it here and what is the difference in this new API and that function?

It is not implemented for columns.
In addition, the repartition condition is also not suitable for workflow that we speed up (repartition occurs when there are 1.5 times more partitions). Due to the lack of such functionality, I have to temporarily use the following hack: pd.DataFrame(try_cast_to_pandas(df)).

[anmyachev]

This has not been tested before when pushing, which affects the stability of codecov results.

[vnlitvinov]

LGTM