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

Method resqml2::AbstractGridRepresentation::getParentCellIndexCount fails #352

Closed
SimoneRinco opened this issue Nov 8, 2024 · 5 comments

Comments

@SimoneRinco
Copy link

What are the steps to reproduce this issue?

Read a dataset which contains an UnstructuredGridRepresentation item with a single value Cell Parent Window attribute.

What does happen?

The method throws an exception with message "This list of cells can only be stored in HDF5 file."

What were you expecting to happen?

The method should return 1.

Any logs, error output, etc?

Any other comments?

This error occurs because a single value cell parent window, set via the setParentWindow method, is not creating an HDF dataset, but storing it as an integer constant array of size 1.
Without knowing how many elements there are, which is the information returned by getParentCellIndexCount(), it is not possible to allocate memory for the output array to pass to getParentCellIndices.
Ideally, we want this value stored in HDF.

What versions of fesapi are you using?

2.8.0

@philippeVerney
Copy link
Member

Thanks for the report.
I'll fix that in the next release : 2.11.1.0 or 2.12 probably. If it's urgent, please let me know.
I don't think to store the value into HDF. Looking at RESQML2.2, I even think the opposite since RESQML2.2 allows to store arrays directly into XML in order to save opening/closing an external binary file for tiny arrays. But I may be wrong.

Finally, I don't expect to release a 2.8.1.0 version with this fix since I cannot afford to maintain too much versions with the community support. I hope this is fine with you. You could think about cherry picking the fix by yourself if needed.

@SimoneRinco
Copy link
Author

Hi Philippe,
it is not urgent. You have a valid point to avoid saving tiny arrays that would require opening/closing files to read a few bytes. However, our code is assuming all data is saved in HDF files. A "small vector" optimisation that can save data directly in XML based on some internal fesapi logic would actually break most of our code.
This is the typical case of application of Hyrum's Law:

"With a sufficient number of users of an API, it does not matter what you promise in the contract: all observable behaviors of your system will be depended on by somebody."

Unfortunately we are somebody.

@philippeVerney
Copy link
Member

I am thinking about adding a configuration field in the EpcExternalPartReference class (aka HdfProxy) to allow FESAPI users to set a limit when they want to write either in XML or in HDF5. For now, there is indeed "some internal FESAPI logic" as this limit has been arbitrarily chosen and hard coded.
You could set it to 0 to always write in HDF5.

More work (it is not only a bug fix now) but probably a valuable one.

@SimoneRinco
Copy link
Author

A configurable limit would be ideal, thanks.

@philippeVerney
Copy link
Member

I just fixed this bug for RESQML2.0.1 in next version.
The configurable limit will have to be done (much later) for RESQML2.2 since RESQML2.0.1 cannot have XML arrays except lattice and constant arrays.

Now getParentCellIndexCount can read all kinds of RESQML arrays (including HDF5 or XML constant ones). And

void setParentWindow(
			unsigned int iCellIndexRegridStart, unsigned int iChildCellCount, unsigned int iParentCellCount,
			unsigned int jCellIndexRegridStart, unsigned int jChildCellCount, unsigned int jParentCellCount,
			unsigned int kCellIndexRegridStart, unsigned int kChildCellCount, unsigned int kParentCellCount,
			class AbstractIjkGridRepresentation* parentGrid, EML2_NS::AbstractHdfProxy * proxy = nullptr, double * iChildCellWeights = nullptr, double * jChildCellWeights = nullptr, double * kChildCellWeights = nullptr);

export constant/XML array of a single element.

void setParentWindow(
			unsigned int iCellIndexRegridStart, unsigned int constantChildCellCountPerIInterval, unsigned int constantParentCellCountPerIInterval, uint64_t iIntervalCount,
			unsigned int jCellIndexRegridStart, unsigned int constantChildCellCountPerJInterval, unsigned int constantParentCellCountPerJInterval, uint64_t jIntervalCount,
			unsigned int kCellIndexRegridStart, unsigned int constantChildCellCountPerKInterval, unsigned int constantParentCellCountPerKInterval, uint64_t kIntervalCount,
			class AbstractIjkGridRepresentation* parentGrid, EML2_NS::AbstractHdfProxy * proxy = nullptr, double * iChildCellWeights = nullptr, double * jChildCellWeights = nullptr, double * kChildCellWeights = nullptr);

export constant/XML array of a single or multiple elements

void setParentWindow(
			unsigned int iCellIndexRegridStart, unsigned int * childCellCountPerIInterval, unsigned int * parentCellCountPerIInterval, uint64_t iIntervalCount,
			unsigned int jCellIndexRegridStart, unsigned int * childCellCountPerJInterval, unsigned int * parentCellCountPerJInterval, uint64_t jIntervalCount,
			unsigned int kCellIndexRegridStart, unsigned int * childCellCountPerKInterval, unsigned int * parentCellCountPerKInterval, uint64_t kIntervalCount,
			class AbstractIjkGridRepresentation* parentGrid, EML2_NS::AbstractHdfProxy * proxy = nullptr, double * iChildCellWeights = nullptr, double * jChildCellWeights = nullptr, double * kChildCellWeights = nullptr);

export HDF5 array of a single or multiple elements (even if the arrays always contain the same constant value)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants