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

options_i command line parsing refactor #1706

Merged
merged 69 commits into from
Jan 16, 2019
Merged

options_i command line parsing refactor #1706

Show file tree
Hide file tree
Changes from 50 commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
dbf5dee
Initial framework commit of options refactor
jackgerrits Nov 30, 2018
45627b0
Rename options -> arguments, option -> parameter
jackgerrits Nov 30, 2018
2e4d6fb
equality unit test
jackgerrits Dec 3, 2018
aa35d8a
Working on tests
jackgerrits Dec 4, 2018
f20f73d
Add kept implementation
jackgerrits Dec 4, 2018
8b3dacc
revert line ending change
jackgerrits Dec 4, 2018
b12c3a9
Update cmake and fix build
jackgerrits Dec 4, 2018
a53364f
Update arg to hold value, expose arg list and merging
jackgerrits Dec 10, 2018
f317655
Rename arguments to options
jackgerrits Dec 14, 2018
196af06
rename files arguments -> options
jackgerrits Dec 14, 2018
c548106
Add config namespace
jackgerrits Dec 17, 2018
f2d8da8
Change back to non-duplicates and use references
jackgerrits Dec 17, 2018
9d443d1
Migration progress:
jackgerrits Dec 17, 2018
056081e
Migrate model header load
jackgerrits Dec 18, 2018
19f6087
Migrate parse_feature_tweaks
jackgerrits Dec 19, 2018
5e9951e
migrate parse_example_tweaks
jackgerrits Dec 19, 2018
ddb0222
migrate parse_output_model and parse_output_preds
jackgerrits Dec 19, 2018
bd42a7e
Change to using reference instead of pointer for options object
jackgerrits Dec 19, 2018
212f57e
Migrate marginal reduction
jackgerrits Dec 19, 2018
201f34a
Change all signatures to include options_i and vw
jackgerrits Dec 20, 2018
de0408f
migrate gd
jackgerrits Dec 20, 2018
3be311e
Migrate kernel svm and fix keep arg in marginal
jackgerrits Dec 21, 2018
020325c
migrate ftrl reduction
jackgerrits Dec 21, 2018
c4ebae5
migrate svrg reduction
jackgerrits Dec 21, 2018
7982425
migrate sender
jackgerrits Dec 21, 2018
969ddd7
migrate gd_mf
jackgerrits Jan 2, 2019
3c88834
work in progress
jackgerrits Jan 2, 2019
1225797
Update loss_function to be able to dynamically identify type
jackgerrits Jan 2, 2019
34f967a
work in progress
jackgerrits Jan 2, 2019
8defa86
Finish migrating reductions, and search tasks
jackgerrits Jan 3, 2019
e0dc05d
fix compile issues in vw_core
jackgerrits Jan 4, 2019
86126ff
working through test fixes
jackgerrits Jan 4, 2019
ed66172
fix bugs
jackgerrits Jan 7, 2019
6e8a7e6
Fix bug, update test model file
jackgerrits Jan 7, 2019
c2204c4
Fix tests, update cppcli wrapper, update c# unit tests, fix c# versio…
jackgerrits Jan 7, 2019
7a1d2be
Fix help printout, unregistered options check
jackgerrits Jan 7, 2019
8a82af8
merge conflicts
jackgerrits Jan 8, 2019
0b9fb19
Fix options leaked object
jackgerrits Jan 8, 2019
02045b1
remove width of print for older boost version
jackgerrits Jan 8, 2019
b52586e
update usage of options in python and remove program_options referenc…
jackgerrits Jan 8, 2019
25628da
add move constructor, fix onethread unregistered issue and fix false …
jackgerrits Jan 8, 2019
bb2fa9e
try not using move constructors
jackgerrits Jan 8, 2019
7f006c1
move to a unique ptr to deal with gcc4.9
jackgerrits Jan 8, 2019
cd07648
fix save resume
jackgerrits Jan 8, 2019
bf8b3f1
trigger ci
jackgerrits Jan 8, 2019
3f7cf1a
implement positional parameter for data and revert test changes
jackgerrits Jan 9, 2019
d3bf228
properly handle invalid args for Java
jackgerrits Jan 9, 2019
ba475f7
fix writing if incorrect cb_type to model
jackgerrits Jan 10, 2019
78c7f2a
fix --opt=val style parsing from model file args
jackgerrits Jan 10, 2019
861fd81
change handling of defaults for cb_type, change insert behavior for e…
jackgerrits Jan 11, 2019
13de7d8
Merge branch 'master' into jagerrit/migrate_options
jackgerrits Jan 11, 2019
31a001c
Fix merge conflict, kvm loss function check is fixed in this PR
jackgerrits Jan 11, 2019
cb0b7c0
revert unwanted changes
jackgerrits Jan 11, 2019
1aaafe7
revert Resource.rc removal
jackgerrits Jan 11, 2019
533f101
revert adding include
jackgerrits Jan 14, 2019
a5528b7
fix kernel_svn -> kernel_svm
jackgerrits Jan 14, 2019
0ef8449
Use const& for strings and larger objects in options
jackgerrits Jan 14, 2019
8a0679c
Fix lgtm alert about copy assignment operator
jackgerrits Jan 14, 2019
9e10531
Merge branch 'master' into jagerrit/migrate_options
jackgerrits Jan 14, 2019
0bea867
add back <algorithm> for min/max
jackgerrits Jan 14, 2019
4dec969
using namespace VW::config;
jackgerrits Jan 15, 2019
89b5959
fix java dependencies, update cpprestsdk build
jackgerrits Jan 15, 2019
9b5215c
Revert "fix java dependencies, update cpprestsdk build"
jackgerrits Jan 15, 2019
6d9cae3
return reference from, rename make function
jackgerrits Jan 15, 2019
f3b022b
remove options.cc
jackgerrits Jan 15, 2019
dce63d8
use given reference for setup_base
jackgerrits Jan 15, 2019
fa7f6d3
remove fields from all
jackgerrits Jan 15, 2019
9337bed
Merge branch 'master' into jagerrit/migrate_options
jackgerrits Jan 16, 2019
2e32eb7
merge commit
jackgerrits Jan 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cs/cli/AssemblyInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ using namespace System::Security::Permissions;
[assembly:AssemblyCopyrightAttribute("Copyright (C) 2014")];
[assembly:AssemblyTrademarkAttribute("Copyright (C) Microsoft Corp 2012-2016, Yahoo! Inc. 2007-2012, and many individual contributors. All rights reserved")];
[assembly:AssemblyCultureAttribute("")];
[assembly:AssemblyVersionAttribute("8.4.0.1")];
[assembly:AssemblyFileVersion("8.4.0.1")];
[assembly:AssemblyVersionAttribute("8.6.1.0")];
[assembly:AssemblyFileVersion("8.6.1.0")];
Binary file modified cs/cli/Resource.rc
View file
Open in desktop
Binary file not shown.
10 changes: 6 additions & 4 deletions cs/cli/vowpalwabbit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -764,8 +764,9 @@ size_t hashstring(String^ s, size_t u)
Func<String^, size_t, size_t>^ VowpalWabbit::GetHasher()
{ //feature manipulation
string hash_function("strings");
if (m_vw->opts_n_args.vm.count("hash"))
{ hash_function = m_vw->opts_n_args.vm["hash"].as<string>();
if (m_vw->options->was_supplied("hash"))
{
hash_function = m_vw->options->get_typed_option<string>("hash").value();
Copy link
Member

Choose a reason for hiding this comment

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

Have you considered removing the extra ->?

Copy link
Member Author

Choose a reason for hiding this comment

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

Which -> are you referring to?

Copy link
Member

Choose a reason for hiding this comment

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

->get_typed_option

Copy link
Member

Choose a reason for hiding this comment

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

Ping on this.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is actually a situation where we need a pointer rather than a reference here. If a command line is supplied we must construct the options_i object for the user, and so we must delete when the vw object is deleted. We could could cast our reference to a pointer in this case but I think we should avoid that. Most usages of options_i in the system is through references

Copy link
Member

Choose a reason for hiding this comment

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

I'm not quite following this. Deleting of members seems routine?

}

if (hash_function == "strings")
Expand Down Expand Up @@ -836,7 +837,7 @@ cli::array<List<VowpalWabbitFeature^>^>^ VowpalWabbit::GetTopicAllocation(int to
auto allocation = gcnew cli::array<List<VowpalWabbitFeature^>^>(K);

// TODO: better way of peaking into lda?
auto lda_rho = m_vw->opts_n_args.vm["lda_rho"].as<float>();
auto lda_rho = m_vw->options->get_typed_option<float>("lda_rho").value();

std::vector<feature> top_weights;
// over topics
Expand All @@ -863,7 +864,8 @@ cli::array<cli::array<float>^>^ VowpalWabbit::FillTopicAllocation(T& weights)
allocation[k] = gcnew cli::array<float>((int)length);

// TODO: better way of peaking into lda?
auto lda_rho = m_vw->opts_n_args.vm["lda_rho"].as<float>();
auto lda_rho = m_vw->options->get_typed_option<float>("lda_rho").value();


for (auto iter = weights.begin(); iter != weights.end(); ++iter)
{ // over topics
Expand Down
33 changes: 21 additions & 12 deletions cs/cli/vw_arguments.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ license as described in the file LICENSE.

#include <msclr\marshal_cppstd.h>
#include "vw.h"
#include "options_serializer_boost_po.h"
#include <algorithm>

using namespace std;
Expand Down Expand Up @@ -40,23 +41,31 @@ public ref class VowpalWabbitArguments
m_finalRegressor(gcnew String(vw->final_regressor_name.c_str())),
m_testonly(!vw->training),
m_passes((int)vw->numpasses)
{ po::variables_map& vm = vw->opts_n_args.vm;
if (vm.count("initial_regressor") || vm.count("i"))
{
auto options = vw->options;

if (vw->initial_regressors.size() > 0)
{ m_regressors = gcnew List<String^>;

vector<string> regs = vm["initial_regressor"].as< vector<string> >();
for (auto& r : regs)
for (auto& r : vw->initial_regressors)
m_regressors->Add(gcnew String(r.c_str()));
}

StringBuilder^ sb = gcnew StringBuilder();
for (auto& s : vw->opts_n_args.args)
sb->AppendFormat("{0} ", gcnew String(s.c_str()));

m_commandLine = sb->ToString()->TrimEnd();

if (vw->opts_n_args.vm.count("cb"))
m_numberOfActions = (int)vw->opts_n_args.vm["cb"].as<uint32_t>();
VW::config::options_serializer_boost_po serializer;
for (auto const& option : options->get_all_options())
{
if (options->was_supplied(option->m_name))
{
serializer.add(*option);
}
}

auto serialized_keep_options = serializer.str();
m_commandLine = gcnew String(serialized_keep_options.c_str());

if (options->was_supplied("cb")) {
m_numberOfActions = (int)options->get_typed_option<uint32_t>("cb").value();
}

m_learning_rate = vw->eta;
m_power_t = vw->power_t;
Expand Down
4 changes: 2 additions & 2 deletions cs/common/Properties/AssemblyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
[assembly: System.Runtime.InteropServices.ComVisible(false)]
[assembly: System.CLSCompliant(false)]
[assembly: System.Runtime.InteropServices.Guid("091c7906-1f69-44d5-a15f-fb29847a68ef")]
[assembly: System.Reflection.AssemblyVersion("8.4.0.1")]
[assembly: System.Reflection.AssemblyFileVersion("8.4.0.1")]
[assembly: System.Reflection.AssemblyVersion("8.6.1.0")]
[assembly: System.Reflection.AssemblyFileVersion("8.6.1.0")]


4 changes: 2 additions & 2 deletions cs/cs/Properties/AssemblyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
[assembly: System.Runtime.InteropServices.ComVisible(false)]
[assembly: System.CLSCompliant(false)]
[assembly: System.Runtime.InteropServices.Guid("6a577997-af00-4ca0-8453-fdc8bbdf2a57")]
[assembly: System.Reflection.AssemblyVersion("8.4.0.1")]
[assembly: System.Reflection.AssemblyFileVersion("8.4.0.1")]
[assembly: System.Reflection.AssemblyVersion("8.6.1.0")]
[assembly: System.Reflection.AssemblyFileVersion("8.6.1.0")]


4 changes: 2 additions & 2 deletions cs/cs_console/Properties/AssemblyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
[assembly: System.Runtime.InteropServices.ComVisible(false)]
[assembly: System.CLSCompliant(false)]
[assembly: System.Runtime.InteropServices.Guid("c7c26e42-6d03-4fe5-943c-add2440f1e37")]
[assembly: System.Reflection.AssemblyVersion("8.4.0.1")]
[assembly: System.Reflection.AssemblyFileVersion("8.4.0.1")]
[assembly: System.Reflection.AssemblyVersion("8.6.1.0")]
[assembly: System.Reflection.AssemblyFileVersion("8.6.1.0")]


4 changes: 2 additions & 2 deletions cs/cs_json/Properties/AssemblyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
[assembly: System.Runtime.InteropServices.ComVisible(false)]
[assembly: System.CLSCompliant(false)]
[assembly: System.Runtime.InteropServices.Guid("8a34db14-bac2-474b-8102-be25ca5f2c55")]
[assembly: System.Reflection.AssemblyVersion("8.4.0.1")]
[assembly: System.Reflection.AssemblyFileVersion("8.4.0.1")]
[assembly: System.Reflection.AssemblyVersion("8.6.1.0")]
[assembly: System.Reflection.AssemblyFileVersion("8.6.1.0")]


4 changes: 2 additions & 2 deletions cs/cs_parallel/Properties/AssemblyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
[assembly: System.Runtime.InteropServices.ComVisible(false)]
[assembly: System.CLSCompliant(false)]
[assembly: System.Runtime.InteropServices.Guid("0bb98c1a-b25f-43a0-94b6-fed77f7e5cd8")]
[assembly: System.Reflection.AssemblyVersion("8.4.0.1")]
[assembly: System.Reflection.AssemblyFileVersion("8.4.0.1")]
[assembly: System.Reflection.AssemblyVersion("8.6.1.0")]
[assembly: System.Reflection.AssemblyFileVersion("8.6.1.0")]


3 changes: 0 additions & 3 deletions cs/unittest/TestArguments.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,6 @@ public void TestArguments()
Assert.IsTrue(vw.Arguments.CommandLine.Contains("--csoaa_ldf multiline"));
Assert.IsTrue(vw.Arguments.CommandLine.Contains("--csoaa_rank"));
Assert.IsTrue(vw.Arguments.CommandLine.Contains("--cb_type ips"));
Assert.IsTrue(vw.Arguments.CommandLine.Contains("--csoaa_ldf multiline"));
Assert.IsTrue(vw.Arguments.CommandLine.Contains("--interact ud"));
Assert.IsTrue(vw.Arguments.CommandLine.Contains("--csoaa_rank"));
}
}

Expand Down
Loading