[Suggestion] add support to use frozen graph

[natxopedreira]

Hello!!

What do you think to add support to use frozen graph? That way you can use a pb created in tf1, so is more compatible.... i'm doing that and works ok. Old cppflow had that option

[danomatika]

If the support exists in cppflow2, you could try it now since the add-on just wraps cppflow classes.

On Sep 13, 2021, at 3:44 PM, natxopedreira @.***> wrote:

Hello!!

What do you think to add support to use frozen graph? That way you can use a pb created in tf1, so is more compatible.... i'm doing that and works ok. Old cppflow had that option

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/zkmkarlsruhe/ofxTensorFlow2/issues/12, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADVK7P4VZ7LIFTQQQ7AII3UBX52LANCNFSM5D53SWFA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

---

Dan Wilcox @danomatika http://twitter.com/danomatika danomatika.com http://danomatika.com/ robotcowboy.com http://robotcowboy.com/

[natxopedreira]

Sorry i don't expressed myself ok.

That feature was not on ccpflow2 but was on cppflow1, i added a pair of methods to cppflow2 and one to ofxTF2Model and now i can use a tf1 frozen graph with your addon.

So i just asking if you can consider to add that feature as it makes more "open" as you are not restricted to savedmodel format.

Its only a suggestion

[danomatika]

Sounds good to me. Supporting some of the previous models can only be good, especially for existing projects which used ofxTensorFlow (1).

Ping @byteosaur

enohp ym morf tnes

Dan Wilcox danomatika.com robotcowboy.com

On Sep 13, 2021, at 5:47 PM, natxopedreira @.***> wrote:

 Sorry i don't expressed myself ok.

That feature was not on ccpflow2 but was on cppflow1, i added a pair of methods to cppflow2 and one to ofxTF2Model and now i can use a tf1 frozen graph with your addon.

So i just asking if you can consider to add that feature as it makes more "open" as you are not restricted to savedmodel format.

Its only a suggestion

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[natxopedreira]

exactly, in fact i see there is a pending pr to cppflow that also do that

[bytosaur]

Hey @natxopedreira, That is in fact a very cool feature and we ll happily review and accept a PR :)

[natxopedreira]

Great! Will prepare it and upload to my fork

[natxopedreira]

I have a question about the preffered style to use.

I need to add a second constructor to cppflow model class to have one constructor to use with frozen and another to savedmodel so both will have the url string as parameter, right now as im in a hurry i added a dummy second parameter to the constructor of frozen and done, but i want to do it ok to send the pr.

I can for example on the "frozen" constructor send TF_Buffer as parameter instead the string with the path or a file...

Ideas on how to deal with the second constructor?

I dont know if im explaining myself ok :)

[danomatika]

Ideas on how to deal with the second constructor?

I dont know if im explaining myself ok :)

Can you add pseudo code here via Markdown?

[natxopedreira]

Sure, here is what im using, you can see the ugly hack of adding the dummy second parameter to model constructor

model.h

inline TF_Buffer *model::readGraph(const std::string& filename) {
        std::ifstream file (filename, std::ios::binary | std::ios::ate);

        // Error opening the file
        if (!file.is_open()) {
            std::cerr << "Unable to open file: " << filename << std::endl;
            return nullptr;
        }

        // Cursor is at the end to get size
        auto size = file.tellg();
        // Move cursor to the beginning
        file.seekg (0, std::ios::beg);

        // Read
        auto data = std::make_unique<char[]>(size);
        file.seekg (0, std::ios::beg);
        file.read (data.get(), size);

        // Error reading the file
        if (!file) {
            std::cerr << "Unable to read the full file: " << filename << std::endl;
            return nullptr;
        }

        // Create tensorflow buffer from read data
        TF_Buffer* buffer = TF_NewBufferFromString(data.get(), size);

        // Close file and remove data
        file.close();

        return buffer;
    }

    inline model::model(const std::string &filename, float p) {
        this->graph = {TF_NewGraph(), TF_DeleteGraph};

        auto session_deleter = [](TF_Session* sess) {
            TF_DeleteSession(sess, context::get_status());
            status_check(context::get_status());
        };

        std::unique_ptr<TF_SessionOptions, decltype(&TF_DeleteSessionOptions)> sess_opts = {TF_NewSessionOptions(), TF_DeleteSessionOptions};

        this->session = {TF_NewSession(this->graph.get(), sess_opts.get(), context::get_status()),session_deleter};
        status_check(context::get_status());

        // Import the graph definition
        TF_Buffer* def = readGraph(filename);
        if(def == nullptr) {
            throw std::runtime_error("Failed to import gragh def from file");
        }

        std::unique_ptr<TF_ImportGraphDefOptions, decltype(&TF_DeleteImportGraphDefOptions)> graph_opts = {TF_NewImportGraphDefOptions(), TF_DeleteImportGraphDefOptions};
        TF_GraphImportGraphDef(this->graph.get(), def, graph_opts.get(), context::get_status());
        TF_DeleteBuffer(def);

        status_check(context::get_status());
    }

ofxTF2Model.h

bool Model::loadGraph(const std::string & modelPath) {
    Model::clear();
    auto model = new cppflow::model(modelPath,0.5);
    if(!model) {
        modelPath_ = "";
        ofLogError("ofxTensorFlow2") << "Model: model could not be initialized!";
        return false;
    }   
    model_ = model;
    modelPath_ = modelPath;
    ofLogVerbose("ofxTensorFlow2") << "Model: loaded model: " << modelPath_;
    return true;
}

[bytosaur]

First of all thanks for your contribution!

If it's a cppflow thing than I think we need to wait for the original PR to happen. I think they did a really good job and maybe they have their reasons for not accepting it yet...

in the meantime we could create a branch that pulls from the fork where the PR comes from and merge your edits to the Model class.

[natxopedreira]

super!!!! but @bytosaur that take a look at the code in pr first.

1) There is code that is not need as is taken directly from the actual constructor and not related in this case.

2) They take code from ccpflow 1 and seting the config_options for session no longer works that way, it will no crash but those will not work, i for example spend hours trying to enable gpu percentage ussage as those options suggested on cppflow1 and the silent fail... and finally find that is not need as current implementation in cppflow2 works

I will suggest make your own branch, use the code cleaned and wait if they solved/merge that one

[bytosaur]

Right, it reminded me of the code for the GPU settings which is not related to the model. I dont think it should belong there even if it works.

so, yeah it s probably best to fork cppflow and edit it on our own.

[natxopedreira]

no it dont work, you dont get any error but no longer works....

[bytosaur]

hey @natxopedreira,

I have forked and modified the model class of cppflow here

I have added a branch containing changes:

• to the ofxTF2::Model class in 55aa572
• and an example in b750781

the model can be downloaded here

let me know what you think :) and thanks for the support

[bytosaur]

argh! i keep getting problems with those submodules...

this solved it however

[main] $ git submodule deinit
[main] $ git checkout 'new_branch'
[new_branch] $ git submodule update --init --recursive

[natxopedreira]

I like it very much! great solution with the enum!!! and i think having support for frozen graph is super cool as you can use tf1 models. I think you should add a note on readme about that

Thanks!!!

[danomatika]

Looks good @byteosaur. I hope the changes can be merged and we can announce support for TF1 models…

enohp ym morf tnes

Dan Wilcox danomatika.com robotcowboy.com

On Sep 16, 2021, at 11:11 AM, natxopedreira @.***> wrote:

 I like it very much! great solution with the enum!!! and i think having support for frozen graph is super cool as you can use tf1 models. I think you should add a note on readme about that

Thanks!!!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.