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

IDataArray or (DataArray) needs meta-data to describe certain kinds of component configurations #281

Open
imikejackson opened this issue Dec 21, 2018 · 13 comments
Assignees

Comments

@imikejackson
Copy link
Contributor

The issue comes to light when a user creates a 6 component array which is really a tensor but currently the XDMF writer assumes it is a pair of 3 RGB vectors. We also have this issue with 2 component arrays. IDataArray could implement a class enumeration to describe the type of data. We would also need to update just about every filter to also set the type and further expose it to the user in the CreateDataArray filter. Possible enumerations are:

Scalar | Vector | Tensor | Tensor6 | Matrix | GlobalID

We could also add RGB | RGBA | COLOR as possibilities.

Which come from this page http://www.xdmf.org/index.php/XDMF_Model_and_Format#Set

We could follow the XDMF naming and call it the "AttributeType" but I am not sure that conveys what it is. "ArrayType" does not really work either as I would think that means float, int,....

ComponentConfiguration, ComponentType?

We could also at least put in the infrastructure but slowly work on updating those filters that specifically need to set a description.

@mmarineBlueQuartz
Copy link
Contributor

mmarineBlueQuartz commented Dec 21, 2018

How about calling it "AttributeCategory"? Category is distinct enough to not be confused with data types like float and int, and Component sounds like it's describing individual component properties, i.e. component (0,0) is a Vector, but component(0,1) is a complete RGB value despite how an RGB value should span three components by itself.

I would shy away from using Array in the enumeration name. Not only is that part of the class name, but every use of array in DataArray.hpp refers to either a DataArray<T>::Pointer or a T* array. It would be better to shorten it to just "Category".

@imikejackson
Copy link
Contributor Author

DataCategory?
ComponentCategory?

@imikejackson
Copy link
Contributor Author

ComponentType

@mmarineBlueQuartz
Copy link
Contributor

VTK also calls this enum AttributeTypes. I'm not sure if that matters to us, but I can live with ComponentType. Looking through DataArray.hpp, the closest we get is component dimensions and getType(). As long as ComponentType and data type are distinct, I don't think we'll be inconsistent.

@imikejackson
Copy link
Contributor Author

I agree. Using AttributeType might be confusing with AttributeArray? AttributeMatrix? ComponetType is pretty close in meaning to what we want. I think we should change the API to be "getDataType()" and "getComponentType()".

Thoughts?

@imikejackson
Copy link
Contributor Author

imikejackson commented Jan 15, 2019

using EnumType = uint32_t;
enum class ComponentType : EnumType
{
  Scalar = 0,  //!<
  Vector = 1,  //!< 3x1 Matrix
  Tensor = 2,  //!< 3x3 Tensor
  Tensor6 = 3,  //!< 6x1 Voight Notation Tensor
  Matrix = 4,  //!< MxN matrix
  GlobalID = 5,  //!< Like a Scalar
  RGB = 6,  //!< 3x1 UChar array
  RGBA = 7,  //!< 4x1 UChar array
  Unknown = 8  //!< 
};

@mmarineBlueQuartz
Copy link
Contributor

mmarineBlueQuartz commented Jan 15, 2019

I am in favor of getDataType() and getComponentType(), though I would suggest changing the description of GlobalID to something other than "Like a Scalar". "size_t identifier", "Integer identification number", "Unique ID", etc.

@imikejackson
Copy link
Contributor Author

We will need to update the API to create Arrays in order to set the ComponentType when creating. Or we can take a guess based on the comp dims that get passed in? This would hit every filter and require manually looking at every filter to ensure the proper component type is being assigned.

@mmarineBlueQuartz
Copy link
Contributor

If we're hitting every filter, can we please move createPrereq* methods to AbstractFilter? It makes far more sense to be a protected method there than public in DataContainerArray, and would clean up the API at the same time. AbstractFilter is creating the prerequisite data in the DataContainerArray, not the other way around.

In addition, by being in AbstractFilter, some of the optimization for renaming can avoid awkward coupling of those two classes by requiring friend status or opening up the API to allow anything to modify a filter's created paths.

@joeykleingers
Copy link
Contributor

I agree with Matthew on that. The filters create the data arrays and simply place them in the DCA; the DCA should not have API that creates data containers, attribute matrices, and data arrays since it is merely acting as the storage container.

@mmarineBlueQuartz
Copy link
Contributor

mmarineBlueQuartz commented Jan 15, 2019

I disagree that the DCA should not have the functionality to create its subcontainers and the data it contains, but that it should not have the create prerequisite functionality as prerequisites have no relevance outside of the scope of what they are prerequisite for. Creating the actual data and subcontainers is still consistent with STL containers like vector and list, after all. Any standard storage container has that capability.

@imikejackson
Copy link
Contributor Author

There was also talk of creating a "Model" class and then that class would have the API to add/remove/modify any of the data structure elements. The API would hand back a "ModelIndex" or "ProxyIndex" to the specific container that was created. This allows things like renaming to happen more fluidly because users of the model are going to access the data structure via a model index and not by the path (Among other advantages)

@joeykleingers
Copy link
Contributor

@mmarineBlueQuartz Fair enough.

@imikejackson Yeah we talked about that at our meeting on New Years Eve, I believe. That was part of our ideas on how to let filters that are farther down the pipeline know that a DC/AM/DA name has changed without having to use the existing N-squared algorithm since it's so slow.

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

No branches or pull requests

6 participants