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

Add log_audio, TBAudio, tests, examples #27

Merged
merged 46 commits into from
May 24, 2019

Conversation

shashikdm
Copy link
Contributor

No description provided.

Shashikant Kadam added 30 commits March 26, 2019 07:54
logger interface updates
change flow of data
different `image_summary` for image object and AbstractArrays
@PhilipVinc
Copy link
Member

Do we really need to ship a 1 Mb .wav file only for testing?
Can't we just generate some random noisy data and be done with it?

@shashikdm
Copy link
Contributor Author

No problem with noisy data. I will remove the test file now

@codecov-io
Copy link

codecov-io commented May 21, 2019

Codecov Report

Merging #27 into master will increase coverage by 1.45%.
The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #27      +/-   ##
==========================================
+ Coverage   79.31%   80.76%   +1.45%     
==========================================
  Files          24       25       +1     
  Lines         290      312      +22     
==========================================
+ Hits          230      252      +22     
  Misses         60       60
Impacted Files Coverage Δ
src/TensorBoardLogger.jl 100% <ø> (ø) ⬆️
src/Loggers/LogAudio.jl 100% <100%> (ø)
src/Loggers/LogImage.jl 100% <100%> (ø) ⬆️
src/Loggers/LogText.jl 96.15% <100%> (+0.32%) ⬆️
src/logger_dispatch_overrides.jl 72.91% <66.66%> (+3.68%) ⬆️
src/protojl/summary_pb.jl 90% <0%> (+10%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c311ed...16d20e3. Read the comment docs.

src/Loggers/LogAudio.jl Outdated Show resolved Hide resolved
@oxinabox
Copy link
Member

@shashikdm a bit off-topic, but you might want to learn to use git rebase.
Rather than repeatedly mergeing in master, rebaseing on top of master,
gives a much cleaner git history and makes reviewing PRs much easier.

@shashikdm
Copy link
Contributor Author

shashikdm commented May 21, 2019

@oxinabox I will take care from next time. Is there any DataType in Julia for sound type variables (like Colorant for Images) that you know of? I looked into https://github.com/JuliaAudio/ but could not find any. Currently log_audio supports AbstractArray of samples. if there's a particular datatype then it will help in automatic dispatch.

@oxinabox
Copy link
Member

You should be able to do the same FileIO thing that was done for Images,
and that will let us also do the same thing to avoid direct dependencies
as in
#28

@oxinabox
Copy link
Member

@oxinabox I will take care from next time. Is there any DataType in Julia for sound type variables (like Colorant for Images) that you know of? I looked into https://github.com/JuliaAudio/ but could not find any. Currently log_audio supports AbstractArray of samples. if there's a particular datatype then it will help in automatic dispatch.

I'm not sure, perhaps @ssfrr will be able to answer?

@shashikdm
Copy link
Contributor Author

yes I had dropped him a dm in slack a few days ago. He has not been online I guess.

@ssfrr
Copy link
Contributor

ssfrr commented May 22, 2019

Yeah, I've been on vacation and not on slack. The JuliaAudio stack stores audio as the SampleBuf type (from SampledSignals.jl, which is basically a wrapper around an AbstractArray that also stores a samplerate.

Let me know if there's any more info you need.

@shashikdm
Copy link
Contributor Author

Great! thank you very much

@shashikdm
Copy link
Contributor Author

So we'll have to add a dependency to SampledSignals.jl. Then we can implement Logger despatch for SampleBuf type variables.

@oxinabox
Copy link
Member

oxinabox commented May 22, 2019

SampledSignals does not have a huge set of dependencies. though it has more than is ideal.
Particularly since I imagine the use of TensorBoard for audio will be rare.

How about we open an issue and think about that later.
See if anyone ends up using TensorBoard logger with this this type.
(and if so, compatibility can come from either package, or from the user's script (though that is technically piracy))

@PhilipVinc
Copy link
Member

Otherwise, we could create a submodule so that to log audio files you must first do using TensorBoardLogger.Audio. We will still need to depend on SampledSignals, but we will only load it and it's dependency if needed.

Though this somewhat defeats the purpose of a general logging framework, because you need to declare in advance that you will be logging audio files...

@oxinabox
Copy link
Member

Lets save working that out for #29 and when it comes up in practice.
Lets get this PR going for now

@PhilipVinc
Copy link
Member

Agreed.
@shashikdm If we leave out the dependency on SampledSignals, what's the status?

@shashikdm
Copy link
Contributor Author

@shashikdm If we leave out the dependency on SampledSignals, what's the status?

log_audios and load_audio is usable for explicit interface.
TBAudios and TBAudio can be used as wrappers for logger interface.
However, there is no automatic dispatch to audio_summary.

@PhilipVinc
Copy link
Member

That's okay for now.
Can I merge?

@shashikdm
Copy link
Contributor Author

Yes, you can merge

@PhilipVinc PhilipVinc merged commit c1dd363 into JuliaLogging:master May 24, 2019
shashikdm pushed a commit to shashikdm/TensorBoardLogger.jl that referenced this pull request May 26, 2019
* use argument decomposition in log_histogram

* Rename Logger to TBLogger

*  [Pulled from NeuralQuantum.jl] Implement the AbstractLogger interface.

* Rename  delta_step -> log_step_increment as suggested by @oxinabox
Add a few docstrings
Remove Coveralls

* Make `preprocess` recursive as suggested by @oxinabox
  - preprocess now calls itself recursively and only pushes to a list if the type can be serialized.
  - Remove the `loggable` function as now it's no longer needed.
  - There is also no need for a stack (and the DataStructures dependency). Just use a Vector for `data` in `handle_message`
  - Fix the readme: `delta_step` -> `log_step_increment`

* Fix tests by using `step` keyword in `log_value`. ( I removed the non-keyword version in the last commit )

* Add very basic test for the logging interface.

* Silently drop things we can't serialize

* Bugfix + Support Tuples

* Log histogram support N-d arrays (#7)

* log N-d arrays

* Update src/Loggers/LogHistograms.jl

Co-Authored-By: shashikdm <[email protected]>

* Update LogHistograms.jl

* Use an enum to describe initialization policy + tests

* Add some tests for scalar and histogram summary creation

* Add coverage

* Add more tests and fix two minor bugs in log_vector

* correct docstring (#13)

* Logtext first implementation (#12)

* init logtext

* Add util function serialize_proto

* Add markdown_repr and removed type constraint on data

* change improved markdown_repr

* Add simple tests for text logger

* minor bug fix

* relax type constraints (#15)

* relax type constraints

* don't loop and push

* A collection of cleanups (#16)

* cleanup text logger

* rename the file with the event code to reflect its contents

* Move the logger type

* cleanup imports and exports

* centeralize the code that decided which logger to use into one file

* - Move proto files out of src/
- Update the README
- remove proto files from coverage

* BugFix

* Log anything that can't be logged otherwise as text

* Split the initialization method of TBLogger

* rename set_step to set_step! and increment_step to increment_step!. Remove old (and unused) methods. Define a reset! method for TBLogger

* Update TBLogger.jl (#18)

* Update TBLogger.jl

minor bug fix for Julia 1.0.3

* Update src/TBLogger.jl

Co-Authored-By: shashikdm <[email protected]>

* Update event.jl

* set default step as a default in make_event (#19)

* set default step as a default in make_event

* Update event.jl

minor typo

* Update .travis.yml (#20)

Update .travis.yml
Test against latest stable version and LTS version
Update test_TBLogger.jl to fix tests on 1.0

* add matrix and list support to log_text

* reverting test_TBLogger.jl

* Wrapper type implementation (#17)

* Wrapper type implementation

* Wrap `propertynames` into `logable_properties` and use it when preprocessing structures so that it's just a one-liner to drop fields out of a struct when logging

* Add common logic to TBHistogram and TBVector so that if they wrap complex arrays they split it into real and imaginary part.

* Add tests

* Add support for Matrix and List in LogText.jl (JuliaLogging#22)

* add matrix and list support to log_text

* Add Compat Intervals

* update readme with install instructions (and correct minor typos)

* Logimage (#21)

Implementing Image logging functionality.

* add `log_image` and `log_images`
* add Pkg.add("Flux") in test
* update Project.toml

* add support for 1-D images in `log_image`

* add ImageFormat explanation

* add seperate function for image objects

* add more formats
change flow of data
different `image_summary` for image object and AbstractArrays

* add preprocess function for image objects for automatic dispatch

* add TBImage, TBImages wrapper

* add tests. more required. more will come

* minor bug fix

* fix some tabspace mix (JuliaLogging#25)

* refactoring

* add support for 3-d images such as mri

* bug fixes

* add more tests

* minor bug fix

* add examples folder

* syntax revision

* change if else to function dispatch `image_summary`

* major revision `log_image` smart use of ImageFormats

* change throw message

* change dict to ternary. minor revision

* add LogAudio.jl with dep WAV.jl

* add test for `log_audio`

* add `TBAudios` and `TBAudio` and test

* bugfix `log_text`

* add examples `Audios.jl` and `Texts.jl`

* Add SummaryCollection constructors that take the summaries (JuliaLogging#26)

* Add SummaryCollection constructors that take the summaries

* use new SummaryCollecton constructor

* change SummaryCollection construction `log_audio`

* add support for 1-D and 3-D images, Logger interface, TBImage Wrapper, Tests, Examples (JuliaLogging#24)

* add matrix and list support to log_text

* add support for 1-D images in `log_image`

* add ImageFormat explanation

* add seperate function for image objects

* add more formats
change flow of data
different `image_summary` for image object and AbstractArrays

* add preprocess function for image objects for automatic dispatch

* add TBImage, TBImages wrapper

* add tests
* add support for 3-d images such as mri
* add more tests

* add examples folder

* syntax revision

* change if else to function dispatch `image_summary`

* major revision `log_image` smart use of ImageFormats

* change throw message

* Update to v 0.1.1

* update

* change docstring

* rm test data

* chane SummaryCollection

* Do not directly depend on ImageMagick (JuliaLogging#28)

* remove direct dependency on ImageMagick
* bump version

* rm WAV dependency

* switch `with_logger` blocks from `begin` to `do` in README (JuliaLogging#30)

* Add `log_audio`, `TBAudio`, tests, examples (JuliaLogging#27)

Logging audio:
* add `TBAudios` and `TBAudio` and their tests
* add examples `Audios.jl` and `Texts.jl`

Improvements to Image logging:
* add support for 1-D images in `log_image`
* add seperate function for image objects
* add dispatching support to image object
* add support for 3-d images such as mrs
* add TBImage, TBImages wrapper

Improvements to Text Logging:
* add matrix and list support to log_text

* switch `with_logger` blocks from `begin` to `do` in README (JuliaLogging#30)
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.

5 participants