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

Smspec c++ #538

Merged
merged 16 commits into from
Nov 30, 2018
Merged

Smspec c++ #538

merged 16 commits into from
Nov 30, 2018

Conversation

joakim-hove
Copy link
Contributor

@joakim-hove joakim-hove commented Nov 18, 2018

Continuation of #526

equinor/libres#458
equinor/ert#167

Note: An important lesson learned while working with this task was the following:

In the old C code a quite common pattern was the following:

my_type * ptr = my_alloc( );

// Insert pointer in vector container, which will enable index based lookup. The 
// vector will alse take ownership of the object and discard it when going out of scope.
vector_insert_owned_ref( storage, ptr, my_free__);

// Insert pointer in hash table, to enable string based lookup. The hash table will not manage
// the underlying object.
hash_insert_ref(hash_table, "KEY", ptr);

This has worked reasonably well - when switching to C++ it was intially tempting to install value objects in a std::vector<my_class> instance, and then access pointers to those from a std::map<my_class *> - that does not work. The point is that when the std::vector<my_class> manages the instances they will not be in stable memory, and things fall apart. This pattern is quite common in librecl/libres!

Steinar Foss added 13 commits November 27, 2018 13:27
smsped_node: added constructor for class.

smspec_node constructor: resolved main issues.

smspec_node_init__ class function.

smspec_node: set_num is a member function.

...

smspec_node: wgname.

smspec_node: decodeR1R2 as member function.

smspec_node: constructor functions v/ statics as member functions.

...

smspec_node: cmp function as member function.

smspec_node: all member functions converted to class members.

smspec _node: wgname made private.

smspec_node: all parameters are private.

...
@joakim-hove joakim-hove force-pushed the smspec_c++ branch 2 times, most recently from 756d370 to c123416 Compare November 27, 2018 14:03
@@ -23,28 +23,13 @@
#include <ert/ecl/ecl_sum.hpp>
#include <ert/ecl/ecl_smspec.hpp>

void test_sort( ecl_smspec_type * smspec )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: this test is removed because the whole ecl_smspec_sort() functionality is removed.

@joakim-hove joakim-hove force-pushed the smspec_c++ branch 2 times, most recently from c1eeec0 to 6326113 Compare November 27, 2018 20:58
@joakim-hove joakim-hove mentioned this pull request Nov 28, 2018
xjules and others added 2 commits November 28, 2018 16:58
Fix to add_node issue, copy smspec_node constructor, args to summary_…
@joakim-hove joakim-hove merged commit c7df730 into equinor:master Nov 30, 2018
@ghost ghost removed the ready for review label Nov 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants