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.

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

augment! and augmentbatch! #10

Merged
merged 5 commits into from
Jul 8, 2017
Merged

augment! and augmentbatch! #10

merged 5 commits into from
Jul 8, 2017

Conversation

Evizero
Copy link
Owner

@Evizero Evizero commented Jul 7, 2017

This allows to augment a full batch at once and store the result into a single higher-dim array (e.g. HxWxN). It also allows for multithreaded use where the images of a batch are processed in parallel. To be able to do that safely I introduced a Mutex to protect access to the global RNG.

@Evizero
Copy link
Owner Author

Evizero commented Jul 8, 2017

@timholy I saw in ImageFiltering.jl that you follow a convention of specifying the ComputationalResources.AbstractResource as the first argument, even for mutating functions (example https://github.com/JuliaImages/ImageFiltering.jl/blob/master/src/imfilter.jl#L117). For consistency I also adopted this convention.

I do wonder though, is there a specific reasoning behind this "overshadowing" of the typical convention of specifying the output variable first?

@timholy
Copy link

timholy commented Jul 8, 2017

It's been a while since I thought that through. If I had a great reason, I can't reconstruct it now. For what it's worth, here are my thoughts:

  • it's kind of in a separate category, and having it compete for space/order with other optional arguments more specific to the particular function seemed fraught. (Check out the dispatch hierarchy in imfilter.jl, which is pretty hairy and took a long time to get right.)

  • it seems a little bit like the convention of putting the function as the first argument with do-block syntax (you're "running it on the resource")

The last is pretty weak. The first might have a bit more merit?

@Evizero
Copy link
Owner Author

Evizero commented Jul 8, 2017

I think both arguments make sense. Thanks for explaining.

as a side note on the first argument: Using the first position is certainly "safer" than last position. I have had problems before by having such optional arguments as the last positional parameter since it basically removed the option to use Vararg sensibly

@Evizero Evizero merged commit 31886c1 into master Jul 8, 2017
@Evizero Evizero deleted the batches branch July 8, 2017 16:43
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