ivoa-std / SpectrumDM

Spectrum Data Model
1 stars 2 forks source link

RFE: order and limits #4

Closed mcdittmar closed 2 years ago

mcdittmar commented 2 years ago

I had expected to make a separate branch for each item, but these effect the same diagram which made that a little more tricky, so I'm submitting them together.

This is an initial pass at satisfying Issues #2 and #3.

Things of note:

lmichel commented 2 years ago

The preview generation is triggered when a commit occurs on the origin/wd-v1.2 branch. We can not trigger it on multiple branch, because the reader wouldn’t know which branch the preview refer to.

The good way to proceed is to work on specific branches (e.g. rfe_order) and to operate PRs on wd-v1.2 (as if it was the main branch) This also facilitate the review since the PR page show you all diffs.

lmichel commented 2 years ago

The mist convenient way to build UCD is to query the UCD builder. You are then pretty sure to get allowed UCDs

http://cdsweb.u-strasbg.fr/UCD/cgi-bin/descr2ucd

Lower limit: "detection lower limit" -> instr.saturation;stat.min Upper limit: "detection upper limit" -> instr.saturation;stat.max

Spectral order "spectral diffraction order" -> stat.variance;instr.order (or just instr.order) Spectral relative order "relative order" -> stat.value;instr.order(to be confirmed by the experts)

mbtaylor commented 2 years ago

Once you've chosen suitable UCDs they should be checked for correctness. You can use Gregory Mantelet's Ucidy or the STILTS harness of it to do that.

It would be nice to include automatic checks for all the UCDs as part of the document build system. I tried this out for EPN-TAP by using a \ucd macro in the LaTeX wherever a checkable UCD was used, then in the Makefile test target pull all the UCDs out using grep and check them using STILTS.

You can see what that looked like at https://github.com/mbtaylor/EPNTAP/commit/3cec868c2; if you like I could submit a PR to do something similar here once the UCDs look somewhat stable.

mcdittmar commented 2 years ago

Thanks Laurent, The spec identifies a very specific file for the UCD vocabulary: (The current list of valid UCDs is http://cdsweb.u-strasbg.fr/UCD/ucd1p-words.txt)

I assumed that list was rather old, where the UCD builder probably uses the latest approved list. So, I simply did a manual look through the list. It turns out:

The mist convenient way to build UCD is to query the UCD builder. You are then pretty sure to get allowed UCDs

http://cdsweb.u-strasbg.fr/UCD/cgi-bin/descr2ucd

Lower limit: "detection lower limit" -> instr.saturation;stat.min Upper limit: "detection upper limit" -> instr.saturation;stat.max

Spectral order "spectral diffraction order" -> stat.variance;instr.order (or just instr.order) Spectral relative order "relative order" -> stat.value;instr.order(to be confirmed by the experts)

mcdittmar commented 2 years ago

I suggest not to merge before the UCD issue is fixed

I think I would prefer addressing the UCDs as a separate RFE/PR, especially if we are going to do more than just specify UCDs for these new element (ala Mark's suggestion of tagging them for automatic validation in the build). I'd like to open an Issue to build the action list for this.

FYI:

lmichel commented 2 years ago

2 points

As you wish to address the UCD question in a separate issue, I can merge the PR.

mcdittmar commented 2 years ago

That looks useful.. I see the test target is added to ivoatex/Makefile, this document has migrated toward, but not fully to using ivoatex. That shouldn't be a problem for the target, but was wondering where the \ucd{} macro spec lives... is that within ivoatex?

Once you've chosen suitable UCDs they should be checked for correctness. You can use Gregory Mantelet's Ucidy or the STILTS harness of it to do that.

It would be nice to include automatic checks for all the UCDs as part of the document build system. I tried this out for EPN-TAP by using a \ucd macro in the LaTeX wherever a checkable UCD was used, then in the Makefile test target pull all the UCDs out using grep and check them using STILTS.

You can see what that looked like at mbtaylor/EPNTAP@3cec868c2; if you like I could submit a PR to do something similar here once the UCDs look somewhat stable.

mbtaylor commented 2 years ago

There's nothing magic about the \ucd macro, you can just define one yourself using \newcommand or whatever (and it can add some distinctive text formatting if you like, but it doesn't have to). The only requirement is that you can write some shell magic in the makefile to pull out the words in the LaTeX that are intended by the authors to be UCDs.

mcdittmar commented 2 years ago

Yes.. I understand this. Since we are trying to make a minimal update to the spec, I did not want to change the referenced UCD list. Changing that to the 'latest and greatest' (or even an approved REC/EN) may invalidate UCDs within the document.

I'll open an Issue for the UCD topic shortly.

2 points

  • The UCD list update is managed with VEPs (https://wiki.ivoa.net/twiki/bin/view/IVOA/VEPsForUCD) as for the vocabularies. We must keep this possibility in mind.
  • The UCD list only contains single words; you must keep in mind that UCDs words can be put together to build composed UCDs.

As you wish to address the UCD question in a separate issue, I can merge the PR.