Reading-eScience-Centre / edal-java

Environmental Data Abstraction Layer libraries
Other
39 stars 30 forks source link

Varying number of edges/nodes per face #94

Closed frizwi closed 6 years ago

frizwi commented 7 years ago

The UGRID spec says to use "_FillValue" when the number of edges/nodes per face is less than the max. However, around line 862 in the "if (fillValue != null) {" block (in CdmGridDatasetFactory.java) after we've searched for the correct value of nEdgesThisFace, we still end up setting it to nEdges - which is the max. Surely this is a bug? Or am I missing something here?

In my case, the dimensions for the face_node_connectivity variable in my netcdf file (attached) are the wrong way around. Which doesn't look like its a problem as there seems to be a swap just above the block that I'm referring to. The only issue is that the index.set() command now fails the check as it doesn't have the swap applied and there is an exception thrown

Finally on a related note, is the Mesh2:face_dimension and Mesh2:edge_dimension supported in EDAL?

Thanks, -Farhan setas.zip

guygriffiths commented 7 years ago

Yep, that looks like a bug, I've fixed it in the develop branch.

face_dimension and edge_dimension are not supported. They weren't part of the UGRID conventions when I implemented this.

frizwi commented 7 years ago

Okay, great thanks.

Cheers,

-Farhan


From: Guy Griffiths notifications@github.com Sent: Wednesday, August 16, 2017 7:00 PM To: Reading-eScience-Centre/edal-java Cc: Rizwi, Farhan (O&A, Hobart); Author Subject: Re: [Reading-eScience-Centre/edal-java] Varying number of edges/nodes per face (#94)

Yep, that looks like a bug, I've fixed it in the develop branch.

face_dimension and edge_dimension are not supported. They weren't part of the UGRID conventions when I implemented this.

- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/Reading-eScience-Centre/edal-java/issues/94#issuecomment-322709838, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AYosE9f-WJn7iJELy77xacIoLwZwv9m5ks5sYq_KgaJpZM4O4ccG.

frizwi commented 7 years ago

Also needs to be fixed around line 990

guygriffiths commented 7 years ago

Thanks, done

hot007 commented 4 years ago

Just hit this bug, has the patch been rolled into production yet and maybe we need to update something?

guygriffiths commented 4 years ago

Yes, this fix should definitely be in the latest release, what version are you using?

frizwi commented 4 years ago

Attaching test file here, in case email response did not work. Using ncWMS 2.4.2, get following exception:

java.lang.IndexOutOfBoundsException: Index: 999999999, Size: 368644 java.util.ArrayList.rangeCheck(ArrayList.java:653) java.util.ArrayList.get(ArrayList.java:429) uk.ac.rdg.resc.edal.dataset.cdm.CdmGridDatasetFactory.generateUnstructuredGridDataset(CdmGridDatasetFactory.java:1015) uk.ac.rdg.resc.edal.dataset.cdm.CdmGridDatasetFactory.generateDataset(CdmGridDatasetFactory.java:120) uk.ac.rdg.resc.edal.dataset.cdm.CdmDatasetFactory.createDataset(CdmDatasetFactory.java:95) uk.ac.rdg.resc.edal.dataset.cdm.CdmDatasetFactory.createDataset(CdmDatasetFactory.java:73) test.nc.gz

guygriffiths commented 4 years ago

Apologies for the long delay in getting around to this. Not sure what happened the fix around line 990, but it wasn't in the develop branch. It is now, and will be in the next release.