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

Defensive checks for ml labels in config #20

Merged
merged 18 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
82 changes: 81 additions & 1 deletion include/config.hh
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
#include <variant>
#include <functional>
#include <unordered_map>
#include <filesystem>
#include <algorithm>
#include <unordered_set>
#include <fstream>

namespace ttool
{
Expand Down Expand Up @@ -203,6 +207,77 @@ namespace ttool
LoadConfigFile();
}

/**
* @brief Check if the acit names match the folder names
*
*/
void CheckAcitFiles(const std::string& TToolRootPath)
{

std::filesystem::path rootPath = std::filesystem::current_path() / TToolRootPath;

std::string line;
std::string toolheadNameTagStart = "<toolhead name=\"";
std::string toolheadNameTagEnd = "\"";

for (const auto& acitFile : m_ConfigData.AcitFiles) {
std::string acitFileR = acitFile.substr(1);
std::filesystem::path acitFilePath = rootPath / acitFileR;
std::string toolheadName = "";

std::ifstream fs(acitFilePath);
if (!fs.is_open()) {
throw std::runtime_error("Could not open file: " + acitFilePath.string());
}

while (std::getline(fs, line)) {
size_t start = line.find(toolheadNameTagStart);
if (start != std::string::npos) {
start += toolheadNameTagStart.length();
size_t end = line.find(toolheadNameTagEnd, start);
if (end != std::string::npos) {
toolheadName = line.substr(start, end - start);
break;
}
}
}
fs.close();

if (acitFile.find(toolheadName) == std::string::npos) {
throw std::runtime_error("Toolhead name mismatch error: Toolhead name \"" + toolheadName +
"\" does not match the folder name \"" + acitFile + "\"");
}
}
}

/**
* @brief Check if the labels in the config file match the file paths of model files and acit files
*
*/
void CheckClassifierLabelsConfig()
{
std::unordered_set<std::string> filePaths;

for (const auto& modelFile : m_ConfigData.ModelFiles) {
filePaths.insert( modelFile);
}
for (const auto& acitFile : m_ConfigData.AcitFiles) {
filePaths.insert(acitFile);
}

for (const auto& label : m_ConfigData.ClassifierLabels) {
bool labelMatches = std::any_of(filePaths.begin(), filePaths.end(),
[&label](const std::string& filePath) {
return filePath.find(label) != std::string::npos;
});

if (!labelMatches) {
throw std::runtime_error("Label mismatch error: Label \"" + label + "\" does not match any file paths");
}
}

}

/**
* @brief Read the config file and set the values to the ConfigData object
*
Expand Down Expand Up @@ -245,10 +320,12 @@ namespace ttool
m_ConfigData.setValue("classifierMean",classifierMean);
m_ConfigData.setValue("classifierStd", classifierStd);

// check the classifier labels match
CheckClassifierLabelsConfig();
return fs.release();
}

/**
/**
* @brief Print the config file to the console
*
*/
Expand Down Expand Up @@ -341,17 +418,20 @@ namespace ttool
*/
ConfigData GetConfigData()
{
std::vector<std::string> fileNames;
// Create a copy of the ConfigData object
ConfigData configData = this->m_ConfigData;
// Prefix the model files with the m_TToolRootPath
for (auto& modelFile : configData.ModelFiles)
{
modelFile = std::string(m_TToolRootPath) + "/" + modelFile;
fileNames.push_back(modelFile);
}
// Prefix the acit files with the m_TToolRootPath
for (auto& acitFile : configData.AcitFiles)
{
acitFile = std::string(m_TToolRootPath) + "/" + acitFile;
fileNames.push_back(acitFile);
}
// Prefix the classifier model path with the m_TToolRootPath
configData.ClassifierModelPath = std::string(m_TToolRootPath) + "/" + configData.ClassifierModelPath;
Expand Down
1 change: 1 addition & 0 deletions include/ttool.hh
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ namespace ttool
TTool(std::string ttoolRootPath, std::string configFile, std::string cameraCalibFile)
{
InitializeConfig(ttoolRootPath, configFile);
m_ConfigPtr->CheckAcitFiles(ttoolRootPath);
Copy link
Collaborator

@9and3 9and3 Jan 17, 2024

Choose a reason for hiding this comment

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

As discussed, I think m_ConfigPtr->CheckAcitFiles(ttoolRootPath); should not be exposed in the constructor.
I propose simply to put it in the definition of InitializeConfig(..) function, in the same ttool.hh header. Here:

https://github.com/ibois-epfl/TTool/blob/6e19a12bcba18326c3bfa8cd76d3e66bd2c27488/include/ttool.hh#L433C9-L438C10

 void InitializeConfig(std::string ttoolRootPath, std::string configFile)
        {
            m_ConfigFile = configFile;
            m_ConfigPtr = std::make_shared<ttool::Config>(configFile);
            m_ConfigPtr->SetTToolRootPath(ttoolRootPath);

            # what I propose
            m_ConfigPtr->CheckAcitFiles(ttoolRootPath);
            m_ConfigPtr->CheckClassifierLabelsConfig();

        }

Let me know if it works, and give it a go on a compiled AC just to be 100% sure that there are no problems.


m_CameraCalibFile = cameraCalibFile;

Expand Down