Skip to content

Commit

Permalink
perf: reduce cppcore initialisation (#410)
Browse files Browse the repository at this point in the history
  • Loading branch information
ilkilic authored Sep 3, 2024
1 parent e9bcd51 commit 6f15c98
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 20 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file.
The format is based on `Keep a Changelog <https://keepachangelog.com/en/1.0.0/>`_,
and this project adheres to `Semantic Versioning <https://semver.org/spec/v2.0.0.html>`_.

5.7.9 - 2024-09
---------------

- Reduces redundant dependency tree initializations in cppcore, improving feature extraction performance by up to 2.5x. Ensures getFeatureValues avoids unnecessary resets; use efel.reset to reload dependencies when needed.

5.6.7 - 2024-08
---------------

Expand Down
8 changes: 6 additions & 2 deletions efel/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,11 @@ def _initialise() -> None:
cppcore.Initialize(_settings.dependencyfile_path, "log")
# flush the GErrorString from previous runs by calling getgError()
cppcore.getgError()
_initSettings()

# Set the settings in the cppcore

def _initSettings() -> None:
"""Init the settings in cppcore."""
settings_attrs = vars(_settings)
for setting_name, setting_value in settings_attrs.items():
if isinstance(setting_value, bool):
Expand Down Expand Up @@ -356,7 +359,8 @@ def _get_feature_values_serial(
else:
raise Exception('stim_start or stim_end missing from trace')

_initialise()
cppcore.Clear()
_initSettings()

# Next set time, voltage and the stimulus start and end
for item in list(trace.keys()):
Expand Down
1 change: 1 addition & 0 deletions efel/cppcore.pyi
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
def Initialize(depfilename: str, outfilename: str) -> int: ...
def Clear() -> int: ...
def getFeature(feature_name: str, values: list) -> int: ...
def getFeatureInt(feature_name: str, values: list[int]) -> int: ...
def getFeatureDouble(feature_name: str, values: list[float]) -> int: ...
Expand Down
7 changes: 7 additions & 0 deletions efel/cppcore/cfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ cFeature::cFeature(const string& strDepFile, const string& outdir)
logger << "Using dependency file: " << strDepFile << endl;
}

void cFeature::clearMap()
{
mapIntData.clear();
mapDoubleData.clear();
mapStrData.clear();
}

template <typename T>
const vector<T> cFeature::getMapData(const string& strName,
const map<string, vector<T>>& mapData) {
Expand Down
35 changes: 18 additions & 17 deletions efel/cppcore/cfeature.h
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
/* Copyright (c) 2015, EPFL/Blue Brain Project
*
* This file is part of eFEL <https://github.com/BlueBrain/eFEL>
*
* This library is free software; you can redistribute it and/or modify it under
* the terms of the GNU Lesser General Public License version 3.0 as published
* by the Free Software Foundation.
*
* This library is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
* FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
* details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this library; if not, write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
/* Copyright (c) 2015, EPFL/Blue Brain Project
*
* This file is part of eFEL <https://github.com/BlueBrain/eFEL>
*
* This library is free software; you can redistribute it and/or modify it under
* the terms of the GNU Lesser General Public License version 3.0 as published
* by the Free Software Foundation.
*
* This library is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
* FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
* details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this library; if not, write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/

#ifndef CFEATURE_H_
#define CFEATURE_H_

Expand Down Expand Up @@ -62,6 +62,7 @@ class cFeature {
string featuretype(string featurename);
string getGError();
void get_feature_names(vector<string>& feature_names);
void clearMap();

};

Expand Down
13 changes: 12 additions & 1 deletion efel/cppcore/cppcore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ int Initialize(const char* strDepFile, const char* outdir) {
}
}

void Clear() {
if (pFeature != NULL) {
pFeature->clearMap();
}
}

static PyObject* CppCoreInitialize(PyObject* self, PyObject* args) {
char *depfilename, *outfilename;
if (!PyArg_ParseTuple(args, "ss", &depfilename, &outfilename)) {
Expand All @@ -64,6 +70,11 @@ static PyObject* CppCoreInitialize(PyObject* self, PyObject* args) {
return Py_BuildValue("");
}

static PyObject* CppCoreClear(PyObject* self, PyObject* args) {
Clear();
return Py_BuildValue("");
}

static vector<int> PyList_to_vectorint(PyObject* input) {
vector<int> result_vector;
int list_size;
Expand Down Expand Up @@ -293,7 +304,7 @@ static PyObject* getgerrorstr(PyObject* self, PyObject* args) {

static PyMethodDef CppCoreMethods[] = {
{"Initialize", CppCoreInitialize, METH_VARARGS, "Initialise CppCore."},

{"Clear", CppCoreClear, METH_NOARGS, "Clear CppCore."},
{"getFeature", getfeature, METH_VARARGS,
"Get a values associated with a feature. Takes a list() to be filled."},
{"getFeatureInt", getfeatureint, METH_VARARGS, "Get a integer feature."},
Expand Down
15 changes: 15 additions & 0 deletions tests/test_cppcore.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,21 @@ def test_caching(self, feature_name):
# make sure Reusing computed value of text occurs twice
assert contents.count(f"Reusing computed value of {feature_name}") == 2

def test_clear_function(self):
"""cppcore: Testing Clear function to reset state"""
import efel
self.setup_data()

feature_values = list()
efel.cppcore.getFeature('AP_amplitude', feature_values)
assert len(feature_values) > 0 # Data should be present

efel.cppcore.Clear()

feature_values = list()
return_value = efel.cppcore.getFeature('AP_amplitude', feature_values)
assert return_value == -1 # Should return -1 since data is cleared


def test_efel_assertion_error():
"""Testing if C++ assertion error is propagated to Python correctly."""
Expand Down

0 comments on commit 6f15c98

Please sign in to comment.