nexusformat / definitions

Definitions of the NeXus Standard File Structure and Contents
https://manual.nexusformat.org/
Other
26 stars 55 forks source link

NXdetector: attribute "link" in several fields #334

Closed prjemian closed 9 years ago

prjemian commented 9 years ago

In NXdetector, the field "time_of_flight" has an attribute "link"

@link: NX_CHAR

    absolute path to location in NXdetector

Why is this attribute needed? Does it provide information that is not already available from the field? Is it different from the "target" attribute used when associating NeXus hard links? Can it be removed?

It is also present in these other fields:

and notably missing in

rayosborn commented 9 years ago

On the subject of x_pixel_offset and y_pixel_offset, I believe that we should also allow ‘x_pixel’ and ‘y_pixel’, i.e., the pixel number starting from [0,0] in the lowest left-hand corner. The main reason is that this is the convention that nearly all other detector software uses (although TIFFs assume the origin is at the top left, so we might need an attribute to declare the direction). It could be argued that this is redundant, because it’s just the array index, but at the moment, we seem to require dimension scales within the NXdata group, and sometimes those are just the array indices. I think we should either remove the requirement for a redundant axis altogether or provide more convenient definitions of them.

P.S. After writing this, I noticed that x_pixel_offset is meant to be in units of length, not just an integer. Perhaps we need a different name than x_pixel to avoid confusion - perhaps x_index - that would imply that x_pixel = x_pixel_size * x_index. Something to discuss in December.

On Oct 7, 2014, at 10:39 AM, Pete R Jemian notifications@github.com<mailto:notifications@github.com> wrote:

In NXdetector, the field "time_of_flight" has an attribute "link"

@link: NX_CHAR

absolute path to location in NXdetector

Why is this attribute needed? Does it provide information that is not already available from the field? Is it different from the "target" attribute used when associating NeXus hard links? Can it be removed?

It is also present in these other fields:

and notably missing in

— Reply to this email directly or view it on GitHubhttps://github.com/nexusformat/definitions/issues/334.

Ray Osborn, Senior Scientist Materials Science Division Argonne National Laboratory Argonne, IL 60439, USA Phone: +1 (630) 252-9011 Email: ROsborn@anl.govmailto:ROsborn@anl.gov

yayahjb commented 9 years ago

Dear Colleagues,

I have no objection to allowing explicit pixel number tags, but tying those tags to the lowest left corner will greatly reduce their utility, because, at least for x-ray crystallography, the origin pixel can be anywhere, lower left, upper right, upper left, lower right, or even at some funny angle -- that is why we have the axis vectors and offsets and the pixel offsets in the file. This is particularly true for the CSPAD, and will be increasingly the case as more multi-module detectors come into use and the modules actually in order detectors start to be used as multi-module detectors.

Regards, Herbert

On Tue, Oct 7, 2014 at 11:56 AM, Ray Osborn notifications@github.com wrote:

On the subject of x_pixel_offset and y_pixel_offset, I believe that we should also allow ‘x_pixel’ and ‘y_pixel’, i.e., the pixel number starting from [0,0] in the lowest left-hand corner. The main reason is that this is the convention that nearly all other detector software uses (although TIFFs assume the origin is at the top left, so we might need an attribute to declare the direction). It could be argued that this is redundant, because it’s just the array index, but at the moment, we seem to require dimension scales within the NXdata group, and sometimes those are just the array indices. I think we should either remove the requirement for a redundant axis altogether or provide more convenient definitions of them.

P.S. After writing this, I noticed that x_pixel_offset is meant to be in units of length, not just an integer. Perhaps we need a different name than x_pixel to avoid confusion - perhaps x_index - that would imply that x_pixel = x_pixel_size * x_index. Something to discuss in December.

On Oct 7, 2014, at 10:39 AM, Pete R Jemian <notifications@github.com mailto:notifications@github.com> wrote:

In NXdetector, the field "time_of_flight" has an attribute "link"

@link: NX_CHAR

absolute path to location in NXdetector

Why is this attribute needed? Does it provide information that is not already available from the field? Is it different from the "target" attribute used when associating NeXus hard links? Can it be removed?

It is also present in these other fields:

  • data
  • data_error
  • x_pixel_offset

and notably missing in

  • y_pixel_offset

— Reply to this email directly or view it on GitHub< https://github.com/nexusformat/definitions/issues/334>.

Ray Osborn, Senior Scientist Materials Science Division Argonne National Laboratory Argonne, IL 60439, USA Phone: +1 (630) 252-9011 Email: ROsborn@anl.govmailto:ROsborn@anl.gov

— Reply to this email directly or view it on GitHub https://github.com/nexusformat/definitions/issues/334#issuecomment-58208036 .

rayosborn commented 9 years ago

Herbert, I agree entirely, which is why I said we might need to add extra attributes, but that was a rushed phrase that I hadn’t thought through properly. I just wanted to say in my comment that I think we need to deal with this issue again.

In the software that we use, there is an orientation matrix that maps the detector storage (x=fast, y=slow) onto the physical coordinate system of the instrument. I couldn’t see any equivalent functionality in the NXdetector group, so I presume it was intended that this is covered by the NXgeometry group. However, that would probably seem overly complicated for many new users.

Is there a reference to how this is handled with CIF? I guess we need to make sure that what we do is at least mappable onto how software like ImageD11 handles this issue.

Ray

On Oct 7, 2014, at 1:03 PM, Herbert J. Bernstein notifications@github.com wrote:

Dear Colleagues,

I have no objection to allowing explicit pixel number tags, but tying those tags to the lowest left corner will greatly reduce their utility, because, at least for x-ray crystallography, the origin pixel can be anywhere, lower left, upper right, upper left, lower right, or even at some funny angle -- that is why we have the axis vectors and offsets and the pixel offsets in the file. This is particularly true for the CSPAD, and will be increasingly the case as more multi-module detectors come into use and the modules actually in order detectors start to be used as multi-module detectors.

Regards, Herbert

On Tue, Oct 7, 2014 at 11:56 AM, Ray Osborn notifications@github.com wrote:

On the subject of x_pixel_offset and y_pixel_offset, I believe that we should also allow ‘x_pixel’ and ‘y_pixel’, i.e., the pixel number starting from [0,0] in the lowest left-hand corner. The main reason is that this is the convention that nearly all other detector software uses (although TIFFs assume the origin is at the top left, so we might need an attribute to declare the direction). It could be argued that this is redundant, because it’s just the array index, but at the moment, we seem to require dimension scales within the NXdata group, and sometimes those are just the array indices. I think we should either remove the requirement for a redundant axis altogether or provide more convenient definitions of them.

P.S. After writing this, I noticed that x_pixel_offset is meant to be in units of length, not just an integer. Perhaps we need a different name than x_pixel to avoid confusion - perhaps x_index - that would imply that x_pixel = x_pixel_size * x_index. Something to discuss in December.

On Oct 7, 2014, at 10:39 AM, Pete R Jemian <notifications@github.com mailto:notifications@github.com> wrote:

In NXdetector, the field "time_of_flight" has an attribute "link"

@link: NX_CHAR

absolute path to location in NXdetector

Why is this attribute needed? Does it provide information that is not already available from the field? Is it different from the "target" attribute used when associating NeXus hard links? Can it be removed?

It is also present in these other fields:

  • data
  • data_error
  • x_pixel_offset

and notably missing in

  • y_pixel_offset

— Reply to this email directly or view it on GitHub< https://github.com/nexusformat/definitions/issues/334>.

Ray Osborn, Senior Scientist Materials Science Division Argonne National Laboratory Argonne, IL 60439, USA Phone: +1 (630) 252-9011 Email: ROsborn@anl.govmailto:ROsborn@anl.gov

— Reply to this email directly or view it on GitHub https://github.com/nexusformat/definitions/issues/334#issuecomment-58208036 .

— Reply to this email directly or view it on GitHub.

Ray Osborn 5928 Northgate Court Woodridge, IL 60517, USA Phone: (630) 515-8418 Email: rayosborn@mac.com

yayahjb commented 9 years ago

The specification of how this is done in cif is in the imgCIF dictionary, with pretty pictures in IUCr International Tables Volume G. The latest released dictionary is at

https://www.sites.google.com/site/nexuscbf/home/cbf-dictionary

There will be an update shortly when I finish untangling the NeXus mapping issues related to the most recent CSPAD CBF sample data.

On Tue, Oct 7, 2014 at 2:22 PM, Ray Osborn notifications@github.com wrote:

Herbert, I agree entirely, which is why I said we might need to add extra attributes, but that was a rushed phrase that I hadn’t thought through properly. I just wanted to say in my comment that I think we need to deal with this issue again.

In the software that we use, there is an orientation matrix that maps the detector storage (x=fast, y=slow) onto the physical coordinate system of the instrument. I couldn’t see any equivalent functionality in the NXdetector group, so I presume it was intended that this is covered by the NXgeometry group. However, that would probably seem overly complicated for many new users.

Is there a reference to how this is handled with CIF? I guess we need to make sure that what we do is at least mappable onto how software like ImageD11 handles this issue.

Ray

On Oct 7, 2014, at 1:03 PM, Herbert J. Bernstein notifications@github.com wrote:

Dear Colleagues,

I have no objection to allowing explicit pixel number tags, but tying those tags to the lowest left corner will greatly reduce their utility, because, at least for x-ray crystallography, the origin pixel can be anywhere, lower left, upper right, upper left, lower right, or even at some funny angle -- that is why we have the axis vectors and offsets and the pixel offsets in the file. This is particularly true for the CSPAD, and will be increasingly the case as more multi-module detectors come into use and the modules actually in order detectors start to be used as multi-module detectors.

Regards, Herbert

On Tue, Oct 7, 2014 at 11:56 AM, Ray Osborn notifications@github.com wrote:

On the subject of x_pixel_offset and y_pixel_offset, I believe that we should also allow ‘x_pixel’ and ‘y_pixel’, i.e., the pixel number starting from [0,0] in the lowest left-hand corner. The main reason is that this is the convention that nearly all other detector software uses (although TIFFs assume the origin is at the top left, so we might need an attribute to declare the direction). It could be argued that this is redundant, because it’s just the array index, but at the moment, we seem to require dimension scales within the NXdata group, and sometimes those are just the array indices. I think we should either remove the requirement for a redundant axis altogether or provide more convenient definitions of them.

P.S. After writing this, I noticed that x_pixel_offset is meant to be in units of length, not just an integer. Perhaps we need a different name than x_pixel to avoid confusion - perhaps x_index - that would imply that x_pixel = x_pixel_size * x_index. Something to discuss in December.

On Oct 7, 2014, at 10:39 AM, Pete R Jemian <notifications@github.com mailto:notifications@github.com> wrote:

In NXdetector, the field "time_of_flight" has an attribute "link"

@link: NX_CHAR

absolute path to location in NXdetector

Why is this attribute needed? Does it provide information that is not already available from the field? Is it different from the "target" attribute used when associating NeXus hard links? Can it be removed?

It is also present in these other fields:

  • data
  • data_error
  • x_pixel_offset

and notably missing in

  • y_pixel_offset

— Reply to this email directly or view it on GitHub< https://github.com/nexusformat/definitions/issues/334>.

Ray Osborn, Senior Scientist Materials Science Division Argonne National Laboratory Argonne, IL 60439, USA Phone: +1 (630) 252-9011 Email: ROsborn@anl.govmailto:ROsborn@anl.gov

— Reply to this email directly or view it on GitHub < https://github.com/nexusformat/definitions/issues/334#issuecomment-58208036>

.

— Reply to this email directly or view it on GitHub.

Ray Osborn 5928 Northgate Court Woodridge, IL 60517, USA Phone: (630) 515-8418 Email: rayosborn@mac.com

— Reply to this email directly or view it on GitHub https://github.com/nexusformat/definitions/issues/334#issuecomment-58233856 .

prjemian commented 9 years ago

back to the original topic:

Is there a reason to keep these "link" attributes in several NXdetector fields?

yayahjb commented 9 years ago

If we definitely assume we are living in an HDF5 world, where all link information can be exposed, I would say the link attributes are redundant.

On Wed, Oct 8, 2014 at 12:39 PM, Pete R Jemian notifications@github.com wrote:

back to the original topic:

Is there a reason to keep these "link" attributes in several NXdetector fields?

— Reply to this email directly or view it on GitHub https://github.com/nexusformat/definitions/issues/334#issuecomment-58387456 .

rayosborn commented 9 years ago

If I'm understanding it correctly, the link attribute should definitely be removed because it duplicates what the target attribute does. I suspect that it is cruft from a long time ago. I think the target attribute is still important because we are hard-linking the items together in HDF5, and within HDF5, there is no concept of a parent item with hard linking. The target attribute has the function of allowing linked data in an NXdata group to be identified with its parent so that other metadata in the NXdetector group can be discovered.

prjemian commented 9 years ago

fixed by 2c0b76b