Skip to content

Conversation

e-strauss
Copy link
Contributor

@e-strauss e-strauss commented Sep 3, 2024

[SYSTEMDS-3744, SYSTEMDS-3745, SYSTEMDS-3746, SYSTEMDS-3747, SYSTEMDS-3748] Python API Builtin cumsum, cumprod, summin, cummax, cumsumprod

This patch adds the builtin operators cumsum, cumprod, summin, cummax, cumsumprod to the python api.

@e-strauss e-strauss force-pushed the MISSING_BUILTIN_CUMSUM branch from 65b8d15 to 6032c72 Compare September 3, 2024 14:17
return Matrix(self.sds_context, "replace", named_input_nodes={"target": self, "pattern": pattern, "replacement": replacement})

def cumsum(self) -> 'Matrix':
""" Column prefix-sum. (For row-prefix sum, use X.t().cumsum().t())
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting, do we not support cumulative sum on rows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

jep, afaik. It says so in the docu (even Arnab told me yesterday not trust it ^^) and java code suggests it too

Copy link
Contributor

Choose a reason for hiding this comment

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

@mboehm7

I think we should probably make a task for this, if it really is not supported.
It seems odd to to two transposes to do row cumulative operations.

Copy link
Contributor

@mboehm7 mboehm7 Sep 4, 2024

Choose a reason for hiding this comment

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

well, it's fine to add a task even through the focus was always R compliance and R defines it as column-wise cumulative aggregates (unary operation without additional parameters). The same also applies to rev(). Recently some of the operations like roll() are more inspired by numpy and there you have indeed usually axis parameters.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also when committing this task here lets call it [SYSTEMDS-3744-48] ... and in the future let's create JIRA tasks for groups of operations that belong together. Thanks.

Copy link
Contributor

@Baunsgaard Baunsgaard Sep 4, 2024

Choose a reason for hiding this comment

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

i have made a task for it and marked it as a student project, unrelated to this PR, such that we do not forget it.
https://issues.apache.org/jira/browse/SYSTEMDS-3762

@e-strauss e-strauss changed the title [SYSTEMDS-3744] Python API Builtin cumsum [WIP] [SYSTEMDS-3744, SYSTEMDS-3745, SYSTEMDS-3746, SYSTEMDS-3747, SYSTEMDS-3748] Python API Builtin cumsum, cumprod, summin, cummax, cumsumprod Sep 4, 2024
@e-strauss e-strauss force-pushed the MISSING_BUILTIN_CUMSUM branch 3 times, most recently from da4f3fa to d69ef96 Compare September 4, 2024 10:19
@e-strauss
Copy link
Contributor Author

the PR should be ready now, I just changed the description of the cumsumprod operator

@e-strauss e-strauss changed the title [WIP] [SYSTEMDS-3744, SYSTEMDS-3745, SYSTEMDS-3746, SYSTEMDS-3747, SYSTEMDS-3748] Python API Builtin cumsum, cumprod, summin, cummax, cumsumprod [SYSTEMDS-3744, SYSTEMDS-3745, SYSTEMDS-3746, SYSTEMDS-3747, SYSTEMDS-3748] Python API Builtin cumsum, cumprod, summin, cummax, cumsumprod Sep 4, 2024
…-3748] Python API Builtin cumsum, cumprod, cumsumprod, cummin, cummax
@e-strauss e-strauss force-pushed the MISSING_BUILTIN_CUMSUM branch from d69ef96 to febadb9 Compare September 4, 2024 14:57
@e-strauss
Copy link
Contributor Author

reformatted

@Baunsgaard Baunsgaard closed this in d3bdb8a Sep 4, 2024
@e-strauss e-strauss deleted the MISSING_BUILTIN_CUMSUM branch September 4, 2024 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants