Add float32 array EclSum.to_numpy #820
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| smspec_node instances. The actual smspec_node instances are | ||
| owned by the smspec_nodes vector; | ||
| */ | ||
| node_map field_var_index; |
There was a problem hiding this comment.
Since this is now moving to the header file, perhaps its a good time to introduce doxygen?
There was a problem hiding this comment.
That's a good idea 👍
| #include <ert/ecl/ecl_smspec.hpp> | ||
| #include <ert/ecl/ecl_sum_tstep.hpp> | ||
| #include <ert/ecl/ecl_file.hpp> | ||
| typedef struct ecl_file_struct ecl_file_type; |
There was a problem hiding this comment.
Maybe change the naming convention to
namespace ecl {
typedef struct fortio_struct fortio;
}
But keep the fortio_type for c code? Possibly use fortio_struct for c code?
There was a problem hiding this comment.
I would probably do something like:
#ifdef __cplusplus
namespace ecl { class file; }
using ecl_file_type = ecl::file;
#else
typedef struct ecl_file_struct ecl_file_type;
#endifIe, change the API to use C++ classes with namespaces, while aliasing them to the old names in C++ only. Should not give us any ABI breakage.
This commit introduces pybind11 in order to accomplish the goal of dumping float32 arrays. This could've been done by adding yet another C function accessible from Python via cwrap. However, I made the decision to have the Python API not depend on the C API directly, instead adding Python bindings via pybind11. This allows us to extend the Python API without exposing implementation details in C and avoids having the Python API do manual memory management to deal with C.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a massive experiment wherein I add a new Python native extension
ecl._nativewritten in C++ using pybind11.I'd like us to use something along these lines for future C++ Pythonic code. My goal also is to have a unified API for Python and C++. That is, my hopes are:
It should be possible to create pybind11 converters between libecl types such as
stringlist_type,std::vector<std::string>andList[str], so that it's easier to avoid using cwrap'sPrototypes in favour of simply specifying the function in pybind.Eg, instead of
we can do this in C++ directly:
The difficulty here is that pybind's C++ classes specifically don't expose Python functionality like
self. That is, they're mostly C++ class wrappers. This is fine if we can convert all the cwrap functionality out of the Python classes.In order to accomplish these things I've bumped the C++ standards version to 17 (up from 11) and CMake to 3.12 (up from 2.8). Will investigate whether we can lower the C++ standard to 14, although I don't particularly want this.
TODO:
ecl_sum_init_double_vectorecl_sum_init_double_vector_interpecl_sum_init_double_frame: Pandas DataFrame versionecl_sum_init_double_frame_interp: Pandas DataFrame versionResolves: #797