mnist_reader_less.hpp: template argument deduction fails

[breznak]

Hi,

we're trying to use your code in our MNIST test, this is a great idea for a repo, btw! Thanks for doing that.
... and the compilation fails with errors in mnist_reader_less.hpp:

In file included from /home/mmm/devel/HTM/htm-community/nupic.cpp/src/examples/mnist/MNIST_SP.cpp:35:
/mnt/store/devel/HTM/htm-community/nupic.cpp/build/ThirdParty/mnist_data/mnist-src/include/mnist/mnist_reader_less.hpp: In function ‘std::vector<_RealType> mnist::read_training_labels()’:
/mnt/store/devel/HTM/htm-community/nupic.cpp/build/ThirdParty/mnist_data/mnist-src/include/mnist/mnist_reader_less.hpp:142:78: error: no matching function for call to ‘read_mnist_label_file<template<class _Tp, class _Alloc> class std::vector>(const char [30])’
return read_mnist_label_filestd::vector("mnist/train-labels-idx1-ubyte");
^
/mnt/store/devel/HTM/htm-community/nupic.cpp/build/ThirdParty/mnist_data/mnist-src/include/mnist/mnist_reader_less.hpp:85:20: note: candidate: ‘template std::vector<_RealType> mnist::read_mnist_label_file(const string&)’
std::vector read_mnist_label_file(const std::string& path) {
^~~~~~~~~~~~~~~~~~~~~
/mnt/store/devel/HTM/htm-community/nupic.cpp/build/ThirdParty/mnist_data/mnist-src/include/mnist/mnist_reader_less.hpp:85:20: note: template argument deduction/substitution failed:
/mnt/store/devel/HTM/htm-community/nupic.cpp/build/ThirdParty/mnist_data/mnist-src/include/mnist/mnist_reader_less.hpp: In function ‘std::vector<_RealType> mnist::read_test_labels()’:
/mnt/store/devel/HTM/htm-community/nupic.cpp/build/ThirdParty/mnist_data/mnist-src/include/mnist/mnist_reader_less.hpp:154:77: error: no matching function for call to ‘read_mnist_label_file<template<class _Tp, class _Alloc> class std::vector>(const char [29])’
return read_mnist_label_filestd::vector("mnist/t10k-labels-idx1-ubyte");
^

This is on g++8, Ubuntu 18.04, -Werror, -Wall

You can see the code and build settings at htm-community/htm.core#242

Thanks for any help resolving the errors.

[wichtounet]

Hi,

Indeed, there is a big bug. This should be fixed with the last commit.

Do you have any reason to use mnist_reader_less.hpp? This file has fewer features than mnist_reader.hpp and has almost never been tested, as shown by your issue. It was made to compile on old compilers. If you are using Linux and a recent compiler, you should probably use mnist_reader.hpp, it has been tested many times.

Cheers

[breznak]

Indeed, there is a big bug. This should be fixed with the last commit.

this is an amazing speed of support, many thanks! 💯 I'll test with the new changes.

Do you have any reason to use mnist_reader_less.hpp? This file has fewer features than mnist_reader.hpp

I'm aware of that, we have a multiplatform framework, so Windows is a part of the mix. I read somewhere in your docs that for Win, the _less.hpp should be used (?) We're using MSVC 2017, so it'd be fine if the "full" reader could be used.

has almost never been tested

I was going to suggest, if your _less API couldn't pull a (compatible) sub-set of features from the "full" API, thus you avoid code duplication and all gets tested.

[wichtounet]

this is an amazing speed of support, many thanks! 💯 I'll test with the new changes.

You are welcome :)

Do you have any reason to use mnist_reader_less.hpp? This file has fewer features than mnist_reader.hpp

I'm aware of that, we have a multiplatform framework, so Windows is a part of the mix. I read somewhere in your docs that for Win, the _less.hpp should be used (?) We're using MSVC 2017, so it'd be fine if the "full" reader could be used.

I honestly do not know if MSVC 2017 can handle the full version. I would indeed advise you to keep with the less version.

has almost never been tested

I was going to suggest, if your _less API couldn't pull a (compatible) sub-set of features from the "full" API, thus you avoid code duplication and all gets tested.

Part of the API is common (mnist_reader_common) and contains the MNIST stuff. So the MNIST parsing has been tested! Only the front-facing API has not been tested enough, I have only used it in one project on Windows.

If you find any issue, I should be able to help you with it.

[breznak]

Tested the latest change and it fixes the issue, many thanks!

If you find any issue, I should be able to help you with it.

There's one more thing,

_less does not provide read_dataset(string path), only the () version. Could you add the path argument, please?

: error: no matching function for call to ‘read_dataset<uint8_t, uint8_t>(std::__cxx11::string)’
dataset = mnist::read_dataset<uint8_t, uint8_t>(string(MNIST_DATA_LOCATION)); //from CMake
^
In file included from /home/mmm/devel/HTM/htm-community/nupic.cpp/src/examples/mnist/MNIST_SP.cpp:36:
/mnt/store/devel/HTM/htm-community/nupic.cpp/build/ThirdParty/mnist_data/mnist-src/include/mnist/mnist_reader_less.hpp:165:29: note: candidate: ‘template<class Pixel, class Label> mnist::MNIST_dataset<Pixel, Label> mnist::read_dataset()’
MNIST_dataset<Pixel, Label> read_dataset() {