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

fix #300 #301

Merged
merged 7 commits into from
Apr 23, 2020
Merged

fix #300 #301

merged 7 commits into from
Apr 23, 2020

Conversation

Roger-luo
Copy link
Contributor

No description provided.

@Roger-luo
Copy link
Contributor Author

I'm not sure where to add a test for this tho.

@Roger-luo
Copy link
Contributor Author

@MikeInnes any chance to review it? I think the CI fails because some other reason.

@MikeInnes
Copy link
Member

Thanks a lot for fixing this! It'd be great if you could tweak it to apply accum_param when using getproperty on a tuple, though, analogous to how we do this for structs. (This patch makes it looks like the tuple itself is the parameter being accumulated, which gets the job done here but could fail in other cases).

@Roger-luo
Copy link
Contributor Author

Like this? @MikeInnes

@cossio
Copy link
Contributor

cossio commented Apr 23, 2020

Merge?

@MikeInnes
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 23, 2020

Build succeeded:

@bors bors bot merged commit d3567e1 into FluxML:master Apr 23, 2020
@MikeInnes
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MikeInnes MikeInnes Awaiting requested review from MikeInnes

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@Roger-luo @MikeInnes @cossio