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

Handle partially supported options manually. #762

Merged
merged 5 commits into from
Aug 23, 2016
Merged

Handle partially supported options manually. #762

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
776ab45
handle partially supported options manually.
qilicun Jul 14, 2016
a9e51ee
treat partially supported options by different multimap.
qilicun Jul 18, 2016
b62834f
Treat unsupported options in a more general way.
qilicun Jul 20, 2016
4ae5a0a
change message type from error to warning.
qilicun Jul 27, 2016
95ab703
Rename addUnsupported to addSupported.
qilicun Aug 22, 2016
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
45 changes: 45 additions & 0 deletions opm/autodiff/MissingFeatures.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,30 @@
#include <opm/autodiff/MissingFeatures.hpp>
#include <unordered_set>
#include <string>
#include <map>


namespace Opm {

namespace MissingFeatures {

template<typename T>
struct PartiallySupported {
int item;
Copy link
Member

Choose a reason for hiding this comment

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

For increased readability I would prefer if you used item names instead - i.e std::string item.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that's fine. My concern is that in the json format for these keywords, the options short cut(that is option name) are somehow different to Eclipse Manual, therefor I choose number instead, if you still think the item name is better, I will change.

Copy link
Member

Choose a reason for hiding this comment

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

My concern is that in the json format for these keywords, the options short cut(that is option name) are somehow different to Eclipse Manual

Yes - I see your point - I would still prefer that you use the string names:

  1. Feel free to submit PR to parser changing the names to the Eclipse names.
  2. Please use the compiled in named constants (we should be better at using these ...):
#include <opm/parser/eclipse/Parser/ParserKeywords/C.hpp>

string_options = { {ParserKeywords::COMPORD::keywordName , PartiallySupported<std::string>{1, ParseKeywords::COMPORD::TRACK::itemName}},

With some template trickery I guess it should be possible just use ParserKeywords::COMPORD and ParserKeywords::COMPORD::TRACK

Copy link
Member

Choose a reason for hiding this comment

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

Untested suggestion:

template <typename Keyword, typename Item,typename T>
void addUnsupported(std::multimap<std::string, PartiallySupported<T>& map, T itemValue) {
    std::pair<std::string,PartiallySupported<T>> pair( Keyword::keywordName , PartiallySupported<T>{ Item::itemName , itemValue});
    map.insert( pair );
}

addUnsupported<ParserKeywords::COMPORD , ParseKeywords::COMPORD::TRACK>( string_options , "TRACK");

}

T item_value;
};

std::multimap<std::string, PartiallySupported<std::string> >
string_options = { {"COMPORD", PartiallySupported<std::string>{1, "TRACK"}},
{"ENDSCALE",PartiallySupported<std::string>{0, "NODIR"}},
{"ENDSCALE",PartiallySupported<std::string>{1, "REVER"}},
{"PINCH", PartiallySupported<std::string>{1, "GAP"}},
{"PINCH", PartiallySupported<std::string>{3, "TOPBOT"}}
};

std::multimap<std::string, PartiallySupported<int> >
int_options = { {"EHYSTR", PartiallySupported<int>{1, 0}}};

void checkKeywords(const Deck& deck)
{
// These keywords are supported by opm-parser, but are not supported
Expand Down Expand Up @@ -66,6 +84,33 @@ namespace MissingFeatures {
+ "In file " + keyword.getFileName() + ", line " + std::to_string(keyword.getLineNumber()) + "\n";
OpmLog::error(msg);
}

// check for partially supported keywords.
std::multimap<std::string, PartiallySupported<std::string>>::iterator string_it, string_low, string_up;
Copy link
Member

Choose a reason for hiding this comment

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

It should be possible to write this scan function as a template over the item type:

template <typename T>
void checkDeck( const Deck& deck , std::multimap<std::string , PartiallySupported<T>>) {
    ....
}

string_low = string_options.lower_bound(keyword.name());
string_up = string_options.upper_bound(keyword.name());
for (string_it = string_low; string_it != string_up; ++string_it) {
const auto& record = keyword.getRecord(0);
if (record.getItem(string_it->second.item).get<std::string>(0) != string_it->second.item_value) {
std::string msg = "For keyword '" + string_it->first + "' only value " + string_it->second.item_value + " in item " +
std::to_string(string_it->second.item + 1) + " is supported by flow.\n"
+ "In file " + keyword.getFileName() + ", line " + std::to_string(keyword.getLineNumber()) + "\n";
OpmLog::error(msg);
}
}

std::multimap<std::string, PartiallySupported<int>>::iterator int_it, int_low, int_up;
int_low = int_options.lower_bound(keyword.name());
int_up = int_options.upper_bound(keyword.name());
for (int_it = int_low; int_it != int_up; ++int_it) {
const auto& record = keyword.getRecord(0);
if (record.getItem(int_it->second.item).get<int>(0) != int_it->second.item_value) {
std::string msg = "For keyword '" + int_it->first + "' only value " + std::to_string(int_it->second.item_value) + " in item " +
std::to_string(int_it->second.item + 1) + " is supported by flow.\n"
+ "In file " + keyword.getFileName() + ", line " + std::to_string(keyword.getLineNumber()) + "\n";
OpmLog::error(msg);
}
}
}
}
} // namespace MissingFeatures
Expand Down