New eclipse data type by markusdregi · Pull Request #1439 · Ensembles/ert

markusdregi · 2017-03-10T10:42:28Z

The first step towards supporting variable string length eclipse keywords (aka. Cxxx). The new data type ecl_data_type is a struct encapsulating the old ecl_type_enum, describing the type of the data, together with the length (that now can vary).

While propagating the new data type, the functionality of the type, both new and old, is moved to ecl_type.c.

joakim-hove · 2017-03-10T11:00:47Z

libecl/include/ert/ecl/ecl_type.h

+
+    // In case of ECL_STRING_TYPE:
+    // length denotes the number of characters in the string
+    const size_t length; 


If we introudcue this structure I think we should use it consistently - i.e. the sizeof_ctype field should be removed from the struct ecl_kw_struct - and:

length should be renamed element_size or something like that.

element_size should be correctly initialized for all the types, i.e.

#define ECL_INT (ecl_data_type) {.type = ECL_INT_TYPE, .element_size = 4}

joakim-hove · 2017-03-10T11:02:10Z

Hmmm - where is the tests/ecl_type_test.c file?:smile:

joakim-hove · 2017-03-10T14:09:56Z

libecl/src/ecl_type.c

+    free(ecl_type);
+}
+
+ecl_data_type ecl_type_get_data_type(const ecl_type_enum type, const size_t element_size) {


This is function creates a new object - i.e. it is not a get function. I would suggest ecl_type / ecl_type_make / ecl_type_create / ....

joakim-hove · 2017-03-10T14:10:54Z

libecl/src/ecl_type.c

+}
+
+void ecl_type_free(ecl_data_type * ecl_type) {
+    if(ecl_type == NULL)


Calling free( NULL ) is a safe no-op.

joakim-hove · 2017-03-10T14:14:42Z

libecl/src/ecl_type.c

+#define ECL_TYPE_NAME_BOOL     "LOGI"
+#define ECL_TYPE_NAME_MESSAGE  "MESS"
+
+ecl_data_type * ecl_type_alloc_copy(const ecl_data_type * src_type) {


As I see it the plan here is to use value semantics - with a publicly available struct definition. I would therefor suggest we just avoid implementing pointer specific functions like ecl_type_alloc_copy( ) and ecl_type_free( ).

joakim-hove · 2017-03-10T14:15:50Z

libecl/src/ecl_type.c

+    return *ecl_type;
+}
+
+ecl_data_type ecl_type_get_data_type_from_type(const ecl_type_enum type) {


It is not get but rather make.

joakim-hove · 2017-03-10T14:17:57Z

libecl/src/ecl_type.c

+}
+
+ecl_data_type ecl_type_get_data_type_from_type(const ecl_type_enum type) {
+    ecl_data_type * ecl_type = NULL;


No reason to go through the pointer, addressof and dereferencing - this should cut it:

switch(type) { case(ECL_CHAR_TYPE) : return ECL_CHAR; case(ECL_INT_TYPE) : return ECL_INT; ...

joakim-hove · 2017-03-17T09:24:50Z

libecl/include/ert/ecl/ecl_kw.h

  ecl_kw_type  * ecl_kw_alloc_empty(void);
  ecl_read_status_enum ecl_kw_fread_header(ecl_kw_type *, fortio_type *);
  void           ecl_kw_set_header_name(ecl_kw_type * , const char * );
+  void           ecl_kw_set_data_type(ecl_kw_type * , ecl_data_type);


Hmmm - should not be necessary with such a set function?

joakim-hove · 2017-03-17T09:28:19Z

libecl/include/ert/ecl/ecl_type.h

+#define ECL_FLOAT (ecl_data_type) {.type = ECL_FLOAT_TYPE, .element_size = sizeof(float)/sizeof(char)}
+#define ECL_DOUBLE (ecl_data_type) {.type = ECL_DOUBLE_TYPE, .element_size = sizeof(double)/sizeof(char)}
+#define ECL_BOOL (ecl_data_type) {.type = ECL_BOOL_TYPE, .element_size = sizeof(int)/sizeof(char)}
+#define ECL_MESS (ecl_data_type) {.type = ECL_MESS_TYPE, .element_size = 1}


element_size = 0for ECL_MESS

joakim-hove · 2017-03-17T09:35:25Z

libecl/include/ert/ecl/ecl_type.h

+typedef struct ecl_type_struct ecl_data_type;
+
+
+#define ECL_CHAR (ecl_data_type) {.type = ECL_CHAR_TYPE, .element_size = 8+1}


sizeof(char) is equal to 1 - by definition.

joakim-hove · 2017-03-17T11:13:33Z

libecl/src/ecl_kw_grdecl.c

+ * Functions only to be used by the *PYTHON* prototype!
+ *
+ */
+ecl_kw_type * python_ecl_kw_fscanf_alloc_grdecl_dynamic__( FILE * stream , const char * kw , bool strict , const ecl_data_type * data_type) {


Make the python a suffix - i.e. ecl_kw_fscanf_alloc_grdecl_dynamic_python__( ) - what a name!

joakim-hove · 2017-03-17T11:25:12Z

libecl/src/ecl_type.c

+
+int ecl_type_get_sizeof_ctype_fortio(const ecl_data_type ecl_type) {
+  if(ecl_type_is_char(ecl_type) || ecl_type_is_C010(ecl_type))
+      return (ecl_type.element_size - 1) * sizeof(char);


sizeof(char) is 1.

joakim-hove · 2017-03-17T11:26:48Z

libecl/src/ecl_type.c

+ * Functions only to be used by the *PYTHON* prototype!
+ *
+ */
+


So many python functions they could get a file of their own.

joakim-hove · 2017-03-17T11:33:03Z

Can we create some wrapping magic in Python so the old enum based way will continue to work, with a big fat warning?

markusdregi · 2017-03-17T13:30:56Z

@joakim-hove: All of your suggestions are implemented

joakim-hove · 2017-03-22T09:41:38Z

libecl/src/ecl_type.c

+    return ecl_type.element_size;
+}
+
+const char * ecl_type_get_type_name(const ecl_data_type ecl_type) {


Can I have the slightly shorter: ecl_type_get_name( )?

made dump file readable by others

joakim-hove · 2017-03-27T06:39:15Z

OK - this is ready to be merged, but it will also affect opm (more than I thought ...) - so the merging must be coordinated.

See: OPM/opm-output#176

version.__repr__ and test eq

* added an explicit flaky test; find out why it occasionally fails

joakim-hove reviewed Mar 10, 2017

View reviewed changes

joakim-hove reviewed Mar 17, 2017

View reviewed changes

markusdregi changed the title ~~WIP: Ecl variable length string~~ WIP: New eclipse data type Mar 21, 2017

markusdregi force-pushed the ecl_string_type branch from 76c1878 to cd2c06f Compare March 21, 2017 12:37

markusdregi changed the title ~~WIP: New eclipse data type~~ New eclipse data type Mar 21, 2017

joakim-hove reviewed Mar 22, 2017

View reviewed changes

joakim-hove mentioned this pull request Mar 22, 2017

Change in ert data_type for ecl_kw. OPM/opm-output#176

Merged

Pål Grønås Drange and others added 5 commits March 22, 2017 11:54

made dump file readable by others

f87a394

Merge pull request Ensembles#1450 from pgdr/ert_abort_dump-chmod

dbd59e7

made dump file readable by others

GridEcl can detect points on cell face

0a45ddf

Containment handles faces/sides

4cf687f

Made the face containment status an enum

e983afc

joakim-hove approved these changes Mar 27, 2017

View reviewed changes

pgdr and others added 3 commits March 27, 2017 09:48

Merge pull request Ensembles#1452 from pgdr/version.__repr__

42900b7

version.__repr__ and test eq

On flaky test runpathlist (Ensembles#1455)

de9d7e5

* added an explicit flaky test; find out why it occasionally fails

Removed empty test-data file.

863beda

markusdregi and others added 27 commits April 3, 2017 11:54

ecl_kw_alloc takes ecl_data_type (Only C-support)

1a4af4c

Various TODO markers

ab77727

EclKw allocates with EclDataType in Python

1dc7560

Updated tests

a25b728

ecl_file_kw use ecl_data_type internally

3224c4d

ecl_kw_fskip_data__ takes ecl_data_type

9f4a94f

ecl_kw is completely ecl_data_type'ified

78a9102

ecl_file_kw uses ecl_data_type

67c7bef

ecl_file_view uses ecl_data_type

9d2cf2e

ecl_file uses new data type

2c5e7b9

ecl_kw_grdecl takes ecl_data_types

0ab2877

EclKw reads grdecl with EclDataType

d5b40fe

libecl has now switched to ecl_data_type

94698a7

Implemented various suggestions from Joakim

6707331

field_config utilizes ecl_data_type

cc3ba32

Field uses the new data type

930e875

gen_data_config uses ecl_data_type

5239e44

gen_common utilizes ecl_data_type

8dc5dab

Complete transfer to ecl_data_type for libenkf

3dbf477

librms uses ecl_data_type

27d5128

Deprecation of EclTypeEnum in python

80079ff

An update of various statoil tests

a09cb2c

Eclipse type enum moved in with Eclipse data type

1a4fc77

Fixed various minor TODO's

f4574f4

Updated some prototypes in ecl_kw.py

eca3dc8

Shortened function names

6b8df21

Fixed some conflicts with master

2958cbb

markusdregi force-pushed the ecl_string_type branch from fe0023e to 2958cbb Compare April 3, 2017 11:00

joakim-hove merged commit 3137b13 into Ensembles:master Apr 3, 2017

bska mentioned this pull request Apr 3, 2017

Add Conditional Support for New ERT Type API OPM/opm-flowdiagnostics-applications#40

Merged

		typedef struct ecl_type_struct ecl_data_type;


		#define ECL_CHAR (ecl_data_type) {.type = ECL_CHAR_TYPE, .element_size = 8+1}

Conversation

markusdregi commented Mar 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joakim-hove commented Mar 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joakim-hove Mar 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joakim-hove Mar 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joakim-hove Mar 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joakim-hove commented Mar 17, 2017

Uh oh!

markusdregi commented Mar 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joakim-hove commented Mar 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

markusdregi commented Mar 10, 2017 •

edited

Loading

joakim-hove Mar 10, 2017 •

edited

Loading

joakim-hove Mar 10, 2017 •

edited

Loading

joakim-hove Mar 17, 2017 •

edited

Loading