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.

Sign up for GitHub

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

Add new geometry definitions #601

Merged
merged 16 commits into from
May 15, 2018
Merged

Add new geometry definitions #601

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
b0d6e98
Add NXoff_geometry definition
Nov 16, 2017
069b070
Correct field description refering to incorrectly named other field
Nov 16, 2017
812875a
Add symbols table to NXoff_geometry
Nov 17, 2017
c28198a
Refine description of coords
Nov 17, 2017
b226432
Add definition of NXcylinderical_geometry
Nov 17, 2017
c537026
Add optional shape description groups to NXdetector
Nov 17, 2017
804eff7
Convert all tabs to space to solve indentation problems, require thre…
Nov 17, 2017
54af60a
Merge branch 'master' into Add_new_geometry_definitions
matthew-d-jones Mar 15, 2018
a650c1b
Improve description of the detector_faces dataset
Mar 15, 2018
e6eb97b
Fix index typo
Apr 12, 2018
67b9581
Reduce line length
Apr 12, 2018
f2a1150
Reduce line length in NXdetector.nxdl.xml
Apr 12, 2018
bbc7776
Fix indentation
Apr 12, 2018
b36d5f7
Fix indentation
Apr 12, 2018
a55aa9c
Use <choice> for pixel_shape and detector_shape options
Apr 12, 2018
3a8073d
Move group name attributes into the choice tag
matthew-d-jones May 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 87 additions & 0 deletions base_classes/NXcylindrical_geometry.nxdl.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
<?xml version="1.0" encoding="UTF-8"?>
<?xml-stylesheet type="text/xsl" href="nxdlformat.xsl" ?>
<!--
# NeXus - Neutron and X-ray Common Data Format
#
# Copyright (C) 2008-2017 NeXus International Advisory Committee (NIAC)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change 2017 to 2018 in the copyright date

#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
# License as published by the Free Software Foundation; either
# version 3 of the License, or (at your option) any later version.
#
# This library is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
# Lesser General Public License for more details.
#
# You should have received a copy of the GNU Lesser General Public
# License along with this library; if not, write to the Free Software
# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
#
# For further information, see http://www.nexusformat.org
-->
<definition xmlns="http://definition.nexusformat.org/nxdl/3.1" category="base"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://definition.nexusformat.org/nxdl/3.1 ../nxdl.xsd"
name="NXcylindrical_geometry"
version="1.0"
type="group" extends="NXobject">

<symbols>
<doc>These symbols will be used below.</doc>
<symbol name="i"><doc>number of vertices required to define all cylinders in the shape</doc></symbol>
<symbol name="j"><doc>number of cylinders in the shape</doc></symbol>
<symbol name="k"><doc>number cylinders which are detectors</doc></symbol>
</symbols>

<doc>
Geometry description for cylindrical shapes.
This class can be used in place of ``NXoff_geometry`` when an exact representation for cylinders is preferred.
For example, for Helium-tube, neutron detectors.
It can be used to describe the shape of any beamline component, including detectors.
In the case of detectors it can be used to define the shape of a single pixel, or,
if the pixel shapes are non-uniform, to describe the shape of the whole detector.
</doc>

<field name="vertices" type="NX_FLOAT" units="NX_LENGTH">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would allow NX_NUMBER. Then you could us mm and store INTs.
Applies a number of times.


<doc>
List of x,y or x,y,z coordinates for vertices. z coordinates are treated as zeroes if omitted.
The origin of the coordinates is the position of the parent component, for example the NXdetector which the geometry describes.
If the shape describes a single pixel for a detector with uniform pixel shape then the origin is the position of each pixel as described by the ``x/y/z_pixel_offset`` datasets in ``NXdetector``.
</doc>

<dimensions rank="2">
<dim index="1" value="i" />
<dim index="2" value="2 or 3" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot define a cylinder in two dimensions.

</dimensions>

</field>

<field name="cylinders">

<doc>
List of indices of vertices in the ``vertices`` dataset to form each cylinder.
Each cylinder is described by three vertices A, B, C.
First vertex A lies on the cylinder axis and circular face, second point B on edge of the same face as A, and third point C at the other face and on axis.
</doc>

<dimensions rank="2">
<dim index="1" value="j" />
<dim index="1" value="3" />
</dimensions>

</field>

<field name="detector_number">

<doc>Maps cylinders in ``cylinder``, by index, with a detector id.</doc>

<dimensions rank="1">
<dim index="1" value="k" />
</dimensions>

</field>

</definition>
20 changes: 20 additions & 0 deletions base_classes/NXdetector.nxdl.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,4 +719,24 @@
subdivision of a detector.
</doc>
</group>
<group name="pixel_shape" type="NXoff_geometry">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My gut feeling would be to wrap the two kinds of pixel_shape in an XSD (or whatever the correct thing is). That makes it obvious that you cannot have both.
To the very least I would group the two next to each other. Same for detector_shape.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe @zjttoefs is correct. A definition with a this-or-that component needs support in the XSD (XML Schema for the NXDL). I'll take a deeper look later today. There may already be a precedent in another definition.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation builds with no errors.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a typo on this line? Perhaps, instead, this:

<dim index="2" value="3" />

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A matter of style in the documentation (in the NXDL files).

Good practice to keep the first documentation to one line and separate from any succeeding lines. This allows automatic generation of concise summaries such as for the base class definitions.

Also, please make a line break between 60-80 characters on a line so that the raw source code becomes much easier to read and review such as this example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering this line:

This dataset should only be used only if the ``NXoff_geometry`` group is describing a detector.

Can we describe the algorithm by which the XML Schema could enforce this? The XML Schema should have no knowledge of any of the NXDL classes. Probably best not to implement in the XSD since that would introduce this knowledge.

Could we enforce this by some rule in the NXDL file itself? Where is the information provided by which validation can check this? By traversing the parent (or chain of parents) of the NXoff_geometry instance?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That the documentation builds is good, but I'm not sure it is sufficient. The validation may need some help to know that there can be one of two groups under a single name. Even just for the person reading the NXDL <choice> make things that little bit more obvious.

<doc>
Shape description of each pixel. Use only if all pixels in the detector are of uniform shape.
</doc>
</group>
<group name="detector_shape" type="NXoff_geometry">
<doc>
Shape description of the whole detector. Use only if pixels in the detector are not of uniform shape.
</doc>
</group>
<group name="pixel_shape" type="NXcylindrical_geometry">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot have two "pixel_shape" groups with the same name. For now this could be an either/or choice, but in the long run we did discuss some scheme of providing alternatives (OFF for the dumb programs, cylinders for the slightly smarter ones, and the CAD thing for whoever wants to implement it - or so).
Probably worth thinking about that some more.

<doc>
Shape description of each pixel. Use only if all pixels in the detector are of uniform shape and require being described by cylinders.
</doc>
</group>
<group name="detector_shape" type="NXcylindrical_geometry">
<doc>
Shape description of the whole detector. Use only if pixels in the detector are not of uniform shape and require being described by cylinders.
</doc>
</group>
</definition>
98 changes: 98 additions & 0 deletions base_classes/NXoff_geometry.nxdl.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
<?xml version="1.0" encoding="UTF-8"?>
<?xml-stylesheet type="text/xsl" href="nxdlformat.xsl" ?>
<!--
# NeXus - Neutron and X-ray Common Data Format
#
# Copyright (C) 2008-2017 NeXus International Advisory Committee (NIAC)
#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
# License as published by the Free Software Foundation; either
# version 3 of the License, or (at your option) any later version.
#
# This library is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
# Lesser General Public License for more details.
#
# You should have received a copy of the GNU Lesser General Public
# License along with this library; if not, write to the Free Software
# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
#
# For further information, see http://www.nexusformat.org
-->
<definition xmlns="http://definition.nexusformat.org/nxdl/3.1" category="base"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://definition.nexusformat.org/nxdl/3.1 ../nxdl.xsd"
name="NXoff_geometry"
version="1.0"
type="group" extends="NXobject">

<symbols>
<doc>These symbols will be used below.</doc>
<symbol name="i"><doc>number of vertices in the shape</doc></symbol>
<symbol name="k"><doc>number of faces in the shape</doc></symbol>
<symbol name="l"><doc>number faces which are detecting surfaces or form the boundary of detecting volumes</doc></symbol>
</symbols>

<doc>
Geometry (shape) description.
The format closely matches the Object File Format (OFF) which can be output by most CAD software.
It can be used to describe the shape of any beamline component, including detectors.
In the case of detectors it can be used to define the shape of a single pixel, or,
if the pixel shapes are non-uniform, to describe the shape of the whole detector.
</doc>

<field name="vertices" type="NX_FLOAT" units="NX_LENGTH">

<doc>
List of x,y or x,y,z coordinates for vertices. z coordinates are treated as zeroes if omitted.
The origin of the coordinates is the position of the parent component, for example the NXdetector which the geometry describes.
If the shape describes a single pixel for a detector with uniform pixel shape then the origin is the position of each pixel as described by the ``x/y/z_pixel_offset`` datasets in ``NXdetector``.
</doc>

<dimensions rank="2">
<dim index="1" value="i" />
<dim index="2" value="2 or 3" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we're saving much by allowing 2D here.

</dimensions>

</field>

<field name="winding_order">

<doc>
List of indices of vertices in the ``vertices`` dataset to form each face, right-hand rule for face normal.
</doc>

<dimensions rank="1">
<dim index="1" value="j" />
</dimensions>

</field>

<field name="faces">

<doc>The start index in ``winding_order`` for each face.</doc>

<dimensions rank="1">
<dim index="1" value="k" />
</dimensions>

</field>

<field name="detector_faces">

<doc>
List of pairs of index in the "faces" dataset and detector id. Used only if describing a detector.
Note, not every face in faces has to be a detecting surface or boundary of detecting volume.
Can use multiple entries with the same detector id to define detector volumes.
</doc>

<dimensions rank="2">
<dim index="1" value="l" />
<dim index="2" value="2" />
</dimensions>

</field>

</definition>