Skip to content

Conversation

@cure1024
Copy link

在compat_pytorch中增加了对math-operations的支持,并添加了相关的单元测试。

@chaoming0625 chaoming0625 self-requested a review February 20, 2023 11:59
Copy link
Member

@chaoming0625 chaoming0625 left a comment

Choose a reason for hiding this comment

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

A good PR. Minor bugs to fix:

  1. alpha is redundant in add() function?
  2. Many functions have incorrect return typing. They should be -> Optional[Array] rather than Array.


def add(input: Union[jax.Array, Array, jnp.number],
other: Union[jax.Array, Array, jnp.number],
*, alpha: Optional[jnp.number] = 1,
Copy link
Member

Choose a reason for hiding this comment

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

where is alpha used?

Copy link
Author

Choose a reason for hiding this comment

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

In Pytorch, the add func is not only "add". It also means "Adds other, scaled by alpha, to input.", and alpha is used in line 134.

Copy link
Member

Choose a reason for hiding this comment

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

Great!

@chaoming0625
Copy link
Member

One more thing, add these functions in brainpy.math.compat_torch.py.

Copy link
Member

@chaoming0625 chaoming0625 left a comment

Choose a reason for hiding this comment

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

Excellent~

@chaoming0625 chaoming0625 merged commit e0a3ee1 into brainpy:master Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants