(r16) PhotDM11RFC < IVOA

IVOA Web>WebPreferences>PhotDM11RFC (revision 16) (raw view)~~EditAttach~~
---+ Photometry Data Model 1.1 Proposed Recommendation: Request for Comments
---++ Introduction

*PHOTDM v1.1 is a revision of 'PhotDM v1.0', redesigned to follow the rules and constraints set by the !VODML meta-model definition.*

The goals are :

   * warrant backward compatibility as much as possible.
   * keep the same concepts as in version !PhotDM v1.0
   * provide the !PhotDMv1-1.vo-dml.xml description of all classes in !VODML format.
All relations between classes and attributed are kept as much as possible.

For backward compatibility, Utypes previously defined in the previous Phot DM 1.0 REC document and listed in appendix B should not be changed after this update.

The benefit is to keep interoperability with the SVO Filterprofile service which has grown to a full Filter repository and represents a kind of Filter registry for the Multi-Wavelength astronomical community.

The new !VODML representation allows building photometric annotations for various kinds of data products: catalogs, spectra, lightcurves, cubes, etc., using the mapping syntax currently proposed in the DM working group.

Bellow attached are the VODML documents representing the data model , and the VODML PhotDM1.1 XML document .

---

The latest version of the model and supporting docs:
   * RCF Document versions (IVOA repository) 
      * [[https://www.ivoa.net/documents/PHOTDM/20220301/index.html][PDF]]
   * <span style="background-color: transparent;">Development Version: (Git Repository) </span> 
      * <span style="color: blue; text-decoration-line: underline; background-color: transparent;">[[https://github.com/ivoa-std/PhotDM][PhotDM github]]</span>
---++ Implementation Requirements
   * [[https://github.com/ivoa-std/MANGO][<strong>MANGO</strong>]] is a proposal for a model designed to enhance the interoperability of catalogue data. The MANGO model is based on native classes the aggregate imported objects among those PhotDM classes are.
   * <strong>VO Model Annotation Syntax </strong>([[https://github.com/ivoa-std/ModelInstanceInVot][VOMAS]]) is a draft for an XML schema allowing to map VOTable data on any model. This draft comes with another repository ([[https://github.com/ivoa/modelinstanceinvot-code][modelinstanceinvot-code]]) that gathers codes that are being developped to exercise the mapping syntax on real data. 2 of these tools are working with PhotDM model: 
      * This project contain a few Jupyter notebooks that can be launched [[https://mybinder.org/v2/gh/ivoa/modelinstanceinvot-code/merge-syntax][online]]
      * *SED Builder*: A [[https://github.com/ivoa/modelinstanceinvot-code/blob/merge-syntax/python/examples/data/NWAYAWGXMM.xml][VOTable]] that gathers photometric data from XMM, 2MASS, WISE and Gaia has been annotated with PhotDM. A demo ([[https://github.com/ivoa/modelinstanceinvot-code/blob/merge-syntax/python/examples/example.6.NWAYAWGXMM.py][standalone]] or [[https://github.com/ivoa/modelinstanceinvot-code/blob/merge-syntax/photdm_impl.ipynb][notebook]] ) reads this VOTABLE and plots one SED fot each data row after having converted MAGs into fluxes when requested.
      * <strong>Serialization of FPS output: </strong>A [[https://github.com/ivoa/modelinstanceinvot-code/tree/merge-syntax/python/scripts][script]] generates XML snippets from FPS output applied to a template. These snippets contain XML serializations of both _PhotDM:PhotmetrySystem_ and <em>PhotDM:</em><em>PhotCal</em> that can be used as components for further VOTable annotations.

---++ Comments from the IVOA Community during RFC/TCG review period: 2022-03-07 - 2022-04-22

The comments from the TCG members during the RFC/TCG review should be included in the next section.

In order to add a comment to the document, please edit this page and add your comment to the list below in the format used for the example (include your Wiki Name so that authors can contact you for further information). When the author(s) of the document have considered the comment, they will provide a response after the comment.

Additional discussion about any of the comments or responses can be conducted on the WG mailing list. However, please be sure to enter your initial comments here for full consideration in any future revisions of this document

   * Sample comment by TWiki.WikiName 
      * Response (by TWiki.WikiName)

---+++ Comments by MarkusDemleitner

Dear Markus,

I have updated the doc on https://github.com/ivoa-std/PhotDM/pull/59 following most of your comments. See responses after your points

-- IVOA.JesusSalgado - 2022-04-11

(1) I have updated the architecture diagram to use variable-width boxes, which also entailed replacing !SpectrumDM (which doesn't exist) with !SpectralDM (that's part of PR #50). While doing that, I had doubts that STC should be listed here -- what we're referencing there is a standard that never really made it and that will most likely be replaced by MCT. Perhaps this should be updated? And there's still Utypes in there, which isn't a standard -- here, I'd suggest dropping it entirely. The caption of the figure with the role diagram might also mention that the specific relationships of !PhotDM to the standards shown is discussed in the introduction.

STC removed from diagram. Architecture diagram caption enhanced.

-- IVOA.JesusSalgado - 2022-04-11

(2) As a general request, it would be great if lines could be reflowed to &lt;80 characters per line, which is helpful for later edits (avoiding unnecessary conflicts; I had to rebase when creating the PR, and believe me, that was painful with these endless lines) and meaningful diffs. I have not done this in the PR to retain meaningful diffs.

Done for almost all the lines (except a few with formulae and URLs that could affect compilation)

-- IVOA.JesusSalgado - 2022-04-11

(3) Please do not use \cite in ivoatexDoc documents. For consistent formatting of the references, use natbib citep or citet as appropriate. PR #50 does this already for the existing references. Also, in the future, please take IVOA documents from ivoabib.bib; that makes it simpler to update references when new versions are issued. I've done this in the PR for IVOA identifiers, Obscore, SDM, and SSAP, which were partly outdated.

(4) Typographically, I'd suggest to write $\langle F\rangle$ rather than $<F>$. This will let !LaTeX choose better spacing, as &lt;&gt; are operators, whereas langle and rangle are delimiters. Also, please do not use paragraphs (well, empty lines) around displayed equations (unless they, indeed, conclude a paragraph, of course), as that, again, leads to bad spacing (see, e.g., the indentation of "so that the effictive flux density"). I have not done this in the PR to keep the diffs reviewable.

Changed to rangle langle

/par from formulae removed

-- IVOA.JesusSalgado - 2022-04-11

(5) Still typographically, units are conventionally witten in roman rather than italic, and most importantly, letter spacing gets all wrong if they are written in math italic. PR #50 introduces macros for the units used in the document and uses them where I've found badly formatted units. Similar considerations apply to function names (ln, sinh).

(6) In my PR, I am also de-inlining some long URLs into footnotes as recommended by ivoatexDoc.

(7) The document mixes British (e.g., "catalogue") and US (e.g., "characterized") spelling. I don't care too much, and the IVOA don't have a policy on this, so perhaps there's no need to worry about this.

Most changed to British English. A language review by a native speaker will be done on a latest stage

-- IVOA.JesusSalgado - 2022-04-11

(8) p. 10, "PhotCal is the class node where !SpectralDM v2.0 interacts." -- since !SpectralDM 2.0 has been shelved and it is not yet clear what will replace it, I'd suggest to take that language out.

Language updated to remove specific connections to Spectral DM (SpectralDM was ongoing when the first version of PhotDM was written)

-- IVOA.JesusSalgado - 2022-04-11

(9) In PR #50, I'm taking out a hard page break on p. 10 which ran against the narrative flow. I suppose you have introduced it to move the floating figure into the chapter body. If that is what you are after, just move the figure environment up a bit lexically. In general, avoid \newpage in attempts to fix the positioning of floating elements -- this will lead to semantically unwelcome page breaks as the document evolves. I give you LaTeX's positioning of floats is sometimes nonintuitive, but it's almost always better to figure out why it is confused rather than forcing it with newpage. In the case of figure 2, you basically forced LaTeX to behave unreasonably be making a page-sized float H. "tph" very typically is a more reasonable placement hint.

(10) I'm not a big fan of the colorbox-es for displayed... examples? In particular, I expect these will be a pain in HTML output. Could you consider whether the listings environment (see ivoatexDoc) works for you, and if not, replace the direct markup with some semantic markup (as in: a custom environment)? Ping me for advice.

Colorbox removed

-- IVOA.JesusSalgado - 2022-04-11

(11) 3.2.2, fpsIdentifier: "The service URL of the filter profile service..." -- the recipe isn't enough to be workable, as a registry record can contain any number of URLs. If you want this to work, you'd at least have to say "Obtain a capability with the standardID suchandsuch and use accessURL(s) given in an XY-interface". From what you write, I don't quite understand how this is intended to work; with some additional information, I can give more specific advice. Of course, if all you're after is associating a URL with the filter, you might consider just dropping this URL into the !PhotometryFilter directly. Finally, the question is: Is there room for more than one FPS in the world? Because if not, we could drop this entirely and canonicalise *the* FPS.

FPS was thought as an open protocol (for filter that cannot be added to the SVO one). We will add a reference to the FPS implementation. <span style="background-color: transparent;">I have modified slightly the text but preventing a full explanation.</span>

<span style="background-color: transparent;">-- IVOA.JesusSalgado - 2022-04-11<br /></span>

(12) 3.2.6 Time validity range: "String time format accepted, ISO8601:" -- since ISO 8601 is such a behemoth, I'd much rather read: "The value is specified as in DALI \citep{2017ivoa.spec.0517D} timestamps."

Change done

-- IVOA.JesusSalgado - 2022-04-11

(13) 3.2.7 "gathered directly as a table using !TransmissionPoint utypes" -- this is, for all I can say, too vague to ensure interoperarbility. If you have an implementation of that, please document more precisely what it produces/expects. If you don't then consider dropping this feature, as having something that's half specificed normally causes a lot more confusion than if it's not specified at all (I'm mentioning utypes themselves as an exemplary case for that).

Reference changed to the FPS serialisation section to preven ambiguities. Reference also added into bibliography

-- IVOA.JesusSalgado - 2022-04-11

(14) 3.2.9.1.1 -- I'm not at all a fan of the "\hspace{0pt} \\" hacks sprinkled in there. If you *really* don't like the default formatting of \paragraph headings, talk to me and I'll work something out.

At this level of hierarchy (5th level) the template does not work very well, without leaving a new line. This trick, at least, help to have a visual good aspect. See point 17

-- IVOA.JesusSalgado - 2022-04-11

(15) 3.2.9.2 transmissionPoint -- In general, the whole section leaves a somewhat unfinished impression, starting with the somewhat clumsy part "Example: em.wl Where em.wl indicates...". Also "The Unit and UCD strings follow specific constraints..." seems a bit superfluous -- and you have not said where the unit string is supposed to end up in. See my point (13) above.

Reference to FPS implementation added to bibliography. We prevent to provide these details into PhotDM

-- IVOA.JesusSalgado - 2022-04-11

(16) 3.2.10.4 (!PhotometryFilter.bandwidth.start, and analogously for .stop): "In practice, this could be taken as the minimum value of the filter transmission curve" -- this would conventionally mean "the minium flux value", which isn't what is meant here. So, perhaps change the text into "the minimum value of the *support* for the filter transmission curve" or similar (well: since the actual support of these functions normally isn't terribly compact, perhas we should rather talk about "a reasonable lower limit on the spectral axis" or so).

Added "reasonable value" for the definition of the minimum and maximum values

-- IVOA.JesusSalgado - 2022-04-11

(17) Many \paragraph items in the document still have hard-coded section numbers, which are quite possibly wrong already, and certainly will be the next time someone touches the document. Please let!LaTeX number these, if you really feel the need to number them. If you need help with that, ping me.

\subsubsubsection is not implemented so they have been replaced by paragraph. This could be redefined as a command into the IVOA LaTeX template

The text follows the recommendation:

https://tex.stackexchange.com/questions/60209/how-to-add-an-extra-level-of-sections-with-headings-below-subsubsection

(First answer on this thread)

-- IVOA.JesusSalgado - 2022-04-11

(18) 3.3 (!PhotCal Class): "Class to describe the use of a photometry filter" -- this is an incomplete sentence; on the following pages, there are quite a few articles missing ("Reference magnitude is a dimensionless variable").

I have added the articles you mentioned although a review by a English native speaker will be done

-- IVOA.JesusSalgado - 2022-04-11

(19) 3.3.1 (!PhotCal.identifier): Couldn't this simply re-use the Photometry Filter identifier? The prose could be a good deal shorter if you just said: "This is the Photometry Filter unique identifier concatenated with a slash and the photometric system type".

The convention you mentioned is the one used by the Spanish VO FPS but it could be a different one (as defined in the reference) so we maintaine the freedom on the identifier construction

-- IVOA.JesusSalgado - 2022-04-11

(20) 3.4.4 ("ZeroPoint.type" and the following sections): "In the !ZeroPoint class we define two conversion functions" -- ummm, no :-). I think you should base the document on the VO-DML structure, and that has no notion of functions or methods. Can't you just pull out the material on these operations into a separate section "Conversion between magnitude and flux using !PhotDM" or so?

That was one of the discussions we had during the PhotDM 1.0 review. PhotDM has an UML diagram behind although most of the IVOA data models are more like ERDs. As you know, ERDs are composed by:
   * Entities
   * Attributes
   * Relationships
And UML diagram are composed by:
   * Classes
   * Attributes
   * Methods
   * Relationships <span style="background-color: transparent;">I think most (almost all) the IVOA DMs are not setting methods because we usually think on IVOA DMs as a way to connect metadata entities that we store into the DB but if you think on the client side implementation, these details are quite important when you want to create a client class extracted from the parsing of a serialised object.</span>
<span style="background-color: transparent;">In summary, there is a lack of support on methods on VO DMs (probably we can modify the term function by method in the text) but this is essential on the data model because ZeroPoint types are extensions of a basic (probably abstract) ZeroPoint class where the methods to convert magnitude to fluxes and vice versa are overrided (and other new attributes could appear). This way to extend mother classes, as you know, is quite general so I think to hide it (because, e.g. we do not know how to represent it in VO/DML) would be a mistake.</span>

<span style="background-color: transparent;">In any case, I think this is out of the scope of the present 1.1 update so it was already decided on the standard 1.0 to go in this way. At PhotDM level, the implementation of the methods should be maintained.</span>

<span style="background-color: transparent;">-- IVOA.JesusSalgado - 2022-04-11<br /></span>

(21) 3.8.2 (!MagnitudeSystem.referenceSpectrum): "This is a URL, pointing to a published IVOA resource location containing the reference spectrum used." -- Umm... what exactly does this mean? Wouldn't it be much better to just say "retrieve the URI to obtain the spectrum"?

Text simplified

-- IVOA.JesusSalgado - 2022-04-11

(22) 4 (Use Cases) Well, as long as there's just one, I'd perhaps use a singular in the section title. But then, in case Ada's time series serialisation is compliant with this (is it?): perhaps that should be the second use case? And if it's not: shouldn't we update that Note then?

Not sure of the status of Ada&rsquo;s time series serialisation so, in order to prevent couplings, I have updated the title of the section to singular

-- IVOA.JesusSalgado - 2022-04-11

(23) A.1 (Zero point magnitude): "Taking the previous equation and clearing the magnitude" -- this is at the very beginning of a major section, and so it is somewhat inappropriate to reference a "previous equation", not to mention that the lexcally preceding equation does not seem to be what the next line derived from (for b!=0). Perhaps for this relatively equation-heavy document you ought to switch on equation numbering anyway?

Equation numbering set with references and added correct reference

-- IVOA.JesusSalgado - 2022-04-11

(24) A.2 (Interrelation between...): "Although, as can be seen in the previous calculation" -- this sentence is missing something like "is possible, that practice" or so.

Text updated to be clearer

-- IVOA.JesusSalgado - 2022-04-11

(25) Appendix B: The over-wide tables with wild hyphenations, too many rules, and run-in page numbers are... not pretty. Do you give me a license to re-format them (which would of course incur breaking the monstrous utypes, but, really, if you insist on the monsters, there's no way around that)?

Still pending on this. We will try to simplify the tables and we we ask for support if we find problems.

About the monsters, I assume this is already in use by some clients and servers so we do not have the freedom to change then into a minor version. Also, they follow the construction rules of other classical DMs

-- IVOA.JesusSalgado - 2022-04-11

(26) Appendix B: "The proposed Utypes are defined following the IVOA rules... from a simplified XML schema" -- well, since that schema, for all I can see, no longer is part of the document (and there are not IVOA rules on utypes), I'd say this information neither helpful nor relevant nor, indeed, correct. It would perhaps be useful to say how these utypes can be derived from the VO-DML if that's possible. But, really, can't you just say: "The utypes given here are to be considered opaque strings [folding case, perhaps?] by consumers of instance documents" and remove all the existing prose from Appendix B? Or am I missing something positive these explanations could do?

I do not remember the origin of this text but, obviously, it was added during the review and now it is not really consistent.

I have simplified in the line you suggested and I have maintained the zero points implementation reflexion that explains exactly the point you raised before (definition on the implementation of functions in a DM is somehow peculiar but it is the only mode found to cover the zero points implementation). As said this point was already discussed at that point and it looks out of the scope of a minor version review

-- IVOA.JesusSalgado - 2022-04-11

(27) Appendix D "Test listing in XML" -- this clearly wasn't intended to be an appendix of its own. And there's some detritus ("begin test syntax highlight") in there that should be removed. Since I'm not sure what this is intended to be, I have not touched anything. But then I'd say anything using dm:mapping shouldn't be in a REC before DM mapping is in late PR at the very least, and so I'd say this thing ought to be removed anyway.

Yes, that was removed

-- IVOA.JesusSalgado - 2022-04-11

(28) Appendix D again: The VOTable given uses the ancient version 1.1 schema. At least that reference (and the version tag) should be updated. A bit of a comment what this is and who should consider, imitate, or whatever that example would also be useful.

Still pending. Iteration needed with FPS developer

-- IVOA.JesusSalgado - 2022-04-11

(29) Appendix D.1 (Photometric Data in Cone Search): I'm really no fan of considerations like this ("could include", "will be as follows"). Mind you, referencing Sébastien's note and commenting if the current update in some way changes what's in there would be highly welcome (and that should be in the introduction). But please decide whether or not you want do define something implementable in there (then make it concrete) or not (then let's please drop it).

I think the language looks ambiguous but the implementation was defined. <span style="background-color: transparent;">So, when it says: could include, it tries to say: For catalogues that include photometric measurements&hellip; </span><span style="background-color: transparent;">And when it says: will be as follows: it tries to say: The workflow to make use of this capability is the following:</span>

<span style="background-color: transparent;">I have modified the text in this line. </span><span style="background-color: transparent;">Just to mention that this capability was integrated into ViZieR in the server side and in VOSpec in the client side as reference implementations.</span>

<span style="background-color: transparent;">-- IVOA.JesusSalgado - 2022-04-11<br /></span>

(30) D.2 "Serialization using VO/DML model": This still has "Test" in the caption and repeats what I have complained about in (27). And I'd again say: Let's avoid any of that while the mapping spec still isn't even a WD.

The Test caption was removed.

About the need or not of this mapping example, it is true that the mapping spec is still a WD but this is a chicken and egg problem as the only way to check that the VO/DML support is doable is through the mapping. The only options I see are either to remove it and go ahead with PhotDM or maintain it with a note on the possible evolution of the mapping. We prefer to maintain it even in the form of an example

Thanks for the review Markus!

-- IVOA.JesusSalgado - 2022-04-11

-- IVOA.MarkusDemleitner - 2022-03-17

---++ Comments from TCG member during the RFC/TCG Review Period:

WG chairs or vice chairs must read the Document, provide comments if any (including on topics not directly linked to the Group matters) or indicate that they have no comment.

IG chairs or vice chairs are also encouraged to do the same, althought their inputs are not compulsory.

---+++ TCG Chair & Vice Chair

---+++ [[IvoaApplications][Applications Working Group]]

---+++ [[IvoaDAL][Data Access Layer Working Group]]

---+++ [[IvoaDataModel][Data Model Working Group]]
---+++ [[IvoaGridAndWebServices][Grid & Web Services]]
---+++ [[IvoaResReg][Registry Working Group]]

---+++ [[IvoaSemantics][Semantics Working Group]]

(1) Semantics is not overjoyed to see em.wl used as the UCD for photDM.PhotometryFilter.transmissionCurve.transmissionPoint.spectralValue.value and in some other places; unless I'm missing something major, the model is built such that the field could contain a frequency or an energy (or a wave number?), too, right? If so, you should either drop the UCD for this field or ask adopters to give whatever applies of em.wl, em.wavenumber, em.freq, or em.energy.

   * This is a good point. The PHOTDM1.0 was intended to represent all filters in the same spectral coordinate, here wavelength, in order to compare the spectral coverage of various filters in one go. The definitions of each attribute of the Bandwidth class (section 3.2.10) allow this already. UCD and units can be specified according to the spectral coordinate chosen to represent the transmission curve. This would then need conversions when when comparing different radio regimes, for instance. -- IVOA.MireilleLouys - 2022-04-08
(2) Similar consideration make me doubt whether pre-setting PhotometryFilter.spectralLocation.UCD to "em.wl" is a good idea. I'd prefer to require deployers to be explicit about this fundamental point. If you follow this reasoning, you should also un-default the spectral units. Personally, I'm not too happy abound having them in the DM in the first place, as you'll have units in the VOTable, too, and specifying the same thing in two places generally is something I'd rather avoid.

   * _The VOTable Format serializes the features defined in the model. In order to compare instruments' various filters in bandwidth and response, it is important to define the spectral nature (freq, wl, energy) and the units for some regimes where they are usually compared. -- IVOA.MireilleLouys - 2022-04-22_
(3) I can't quite tell from the document how PhotCal.magnitudeSystem.ReferenceSpectrumURI is intended to be resolved, but I have a hard time figuring out how making this a meta.ref.ivoid (as opposed to meta.ref.uri or perhaps url) could be right -- you're not planning to store spectra in the Registry, are you?

   * _agreed , no need for a an ivoid. Changed as meta.ref.url. -- IVOA.MireilleLouys - 2022-04-22_
(4) I think I'm also not too happy about calling the asinh softening parameter an obs.param. Isn't this more a parameter of the data reduction (if anything)? I give you stat.fit.param isn't all ideal for that either, but it's more plausible than obs.param in my book. Or what about phot.calib? I suppose my vote would be for <generic parameter>;phot.calib if there were a generic parameter atom...

   * in this intermediate version 1.1, we aim at keeping backward compatibility. A better term will be choosen for PhotDM2.0. _-- IVOA.MireilleLouys - 2022-04-22_
-- IVOA.MarkusDemleitner - 2022-03-17

---+++ [[IvoaTheory][Education Interest Group]]

---+++ [[IvoaKDD][Knowledge Discovery Interest Group]]

---+++ [[IvoaSS][Solar System Interest Group]]

I'm not a spectrscopist, so mainly I'm reading this from the point of view of someone who maintains spectral metadata for my archive and would be looking to try to map this model to my own. From that point of view the method and approach seem clear, and my comments, consequently, are mainly about readability and clarity of description.

   * Section 2, page 8, first paragraph. The sentence beginning "mR is most often chosen..." has too many instances of the words "most" and "usually". Combined with the parenthetical remark, the net effect is to render the sentence nearly meaningless (e.g.: "mR is most often chosen to be zero, except when it isn't, and even when it is zero it isn't zero because the system changes."). A rewriting as a set of simpler, direct statements might be helpful.
A simplification of the sentence has been added following the spirit of the original text

-- IVOA.JesusSalgado - 2022-04-25

   * Section 3.8.1: The statements "Possible values are:" and "The list is not exhaustive" seem to contradict each other. If the list is not exhaustive, then these would be better described as "example values". A reference to the location of the current exhaustive list of valid values would be helpful. Also, there is a reference saying "See section 2.2.3 for a detailed description," but there is no section 2.2.3 in this document and no other document is named.
Done and reference updated

-- IVOA.JesusSalgado - 2022-04-25

   * Section 3.8.2, third paragraph: I suspect this paragraph (beginning "This URL, ...") should instead be a continuation of the previous paragraph (otherwise "This" is undefined). It might even have been intended to be a parenthetical clarification of "referenceSpectrumURI". In fact, all five of these single-sentence paragraphs would be better formulated as a single paragraph. I am not able to make any sense of the end of the fourth paragraph, starting at about "...for instance or re-use Curation.PublisherDID which is a...". There is certainly some punctuation missing, but has a word or two has been dropped as well? Or maybe there's a reference to another model/standard that is missing? I am not sure what "This mechanism" at the beginning of the fifth paragraph is referring to. The examples that follow do not include URIs, and thus do not help to demystify the text.
Previous text tries to use a registration/discovery system that was not used at the end and complicates the access. Example added and text simplified

-- IVOA.JesusSalgado - 2022-04-25

   * Section A.1 refers to "Taking the previous equation", but there is no previous equation in either section A.1 or Section A. References to equations that depend on document order tend to be problematic when documents are updated. I recommend numbering the equation and referring to the number instead of the current relative position in the document.
Already done as it was also recommended by other reviewers

-- IVOA.JesusSalgado - 2022-04-25

   * Section B: This table is pretty hard to use. In the PDF, the page numbers occur in the middle of the first column, and despite the wideness of the table some values still wrap, while others overwrite each other. The "Meaning" and "Default Value" columns are particularly difficult to disentangle from each other. Given the point size of type considered acceptable in Figures 1 and 2, it seems like a better solution might be obtainable here...
Tables updated

-- IVOA.JesusSalgado - 2022-04-25

   * Section C: The indenting seems to follow no specific pattern. Is it meaningful? If not, why bother?
The only intention at this stage is to produce an output that could be read easily. Still some work on this area is ongoing

-- IVOA.JesusSalgado - 2022-04-25

Typos:

   * Section 3.2.5: "discourage" should be "discouraged".
   * Section 3.2.7, third paragraph: "modifies" should be "modify".
   * Section 3.2.7, last sentence: There is an extra '.' at the end of the last sentence.
   * Section 4.1, second bullet: Should "photdm:PhotCal.identifie" be "photdm:PhotCal.identifier"?
   * Section B, page 35, second paragraph: There seems to be a spurious line break before "However, the data structure...".
   * Section B, page 35, last paragraph: "using array" should be either "using an array" or "using arrays".
   * Section C1, page 36, last FIELD: I suspect 'utype= T"photDM...' should be 'utype="photDM...".
   * Section C3, page 45, "Transmision Curve" should be "Transmission Curve" (in XML comment)
Typos corrected. Thanks for the review Anne

-- IVOA.JesusSalgado - 2022-04-25

-- IVOA.AnneRaugh - 2022-04-20
---+++ [[IvoaTheory][Theory Interest Group]]

---+++ [[IvoaVOEvent][Time Domain Interest Group]]

The following is a review of the PDF dated 2022-04-22 from the Git repository, which appears to include the twiki items dated 2022-04-25.

Overall.. nice job with the conversion to !VODML. I think the comments below look worse than they are. I consider them rather minor details which may help avoid confusion in future readers of the document.

Question: Is there a schema file for the model? I don't see one in the repository.

*Typo-s:*

   * Section 3, paragraph 7: &ldquo;PhotCal is the class node where data model describing spectral data interacts&rdquo; 
      * Removal/Substitution of &ldquo;Spectral DM V2.0&rdquo; here left the sentence a bit non-sensical 
         * &ldquo;where another data model describing spectral data interacts&rdquo;
         * &ldquo;where a data model describing spectral data interacts&rdquo;
         * &ldquo;where a spectral data model interacts&rdquo;
      * extra space.. &ldquo;the calibration configuration used , bringing&rdquo;
   * Section 3.1.2: &ldquo;At current state, this list is exhaustive&rdquo; 
      * Not the best phrasing, but not bad.. &ldquo;At present, &ldquo; or &ldquo;At the time of this writing, &ldquo; might be more clear.
   * Section 3.2.7: &ldquo;serialised&rdquo; -&gt; &ldquo;serialized&rdquo;
   * Section 3.2.10.4: &ldquo;Also called in the rest of the document&rdquo;.. lost lambda-min
   * Section 3.2.10.5: the formula shows &ldquo;min = &ldquo; rather than &ldquo;max = &ldquo; was OK in V1.0
   * Section 3.8: Tables missing vertical bars
   * Section 4: 
      * bullet 1 - extra space in canonical utype declaration &ldquo; spec:Spectrum.Data.FluxAxis.value&rdquo;
      * bullets 2,4 - uses &ldquo;photdm:&rdquo; for the !UType 'namespace' rather than &ldquo;photDM:&rdquo; in the UType table and elsewhere in the text. 
         * NOTE - this is true in V1.0 as well, but should maybe be corrected.
         * NOTE - Appendix C.3 also uses &ldquo;photdm:&rdquo; for the vodml-id namespace, but this could actually be different.
*Diagram errors/inconsistent with text:*

   * Section 3.2.2 - &ldquo;PhotometryFilter.fpsIdentifier&rdquo; is &ldquo;fpsidentifier&rdquo; in the diagram. (lower case &lsquo;I&rsquo;)
   * Section 3.2.5 - &ldquo;PhotometryFilter.bandName&rdquo; is &ldquo;bandname&rdquo; in diagram. (lower case &ldquo;N&rdquo;)
*ISOTime type*:

   * Section 3.2.6: &ldquo;Validity time stamps are expressed as !ISOTime as specified in DALI (Dowler and Demleitner et al., 2017) timestamps.&rdquo; 
      * This is a pseudo-type for "string of a particular format", I don't see a definition of an !ISOTime type in either V1.0 or V1.1. 
         * so use of it as a 'type' in the diagrams and text may lead to confusion.
         * the !UType table shows these elements as 'string' type.
      * NOTE: this definition is consistent with the Coords model &ldquo;ISOTime&rdquo; type. The Coords model does not reference DALI, but the language is intentionally aligned with the referenced DALI content. In Coords, I&rsquo;ve used the ivoa:datetime type for the base value type.. rather than 'ivoa:string'.
*UCD type:*

   * Figure 2: diagram shows !SpectralLocation.ucd, Bandwidth.ucd, !TransmissionPoint.ucd, Flux.ucd as &ldquo;UCD&rdquo; type, which is not a defined type. 
      * similar to !ISOTime, the text describes these as a String type with specific constraints (complies with UCD standard). 
         * Section 3.2.9.2.1: !TransmissionPoint.spectralValue.UCD == String type with specific constraints (UCD)
         * Section 3.2.10.1: !PhotometryFilter.bandwidth.UCD == String type with specific constraints (UCD)
         * !UType Table: UCD elements of type &lsquo;string&rsquo;
   * Appendix C.3 - VODML serialization: dmtype=&ldquo;ivoa:UCD&rdquo;, which is not a thing.
*PhysicalQuantity:*

<span style="white-space: pre;"> </span>This version appears to have dropped the !PhysicalQuantity types, and dispersed the attributes into objects.<br /><span style="white-space: pre;"> </span>The !UTypes for the endpoints are unchanged, so I suppose the thought is that this is sufficient for backward compatibility..
   * 
      * Examples: 
         * !ZeroPoint.referenceMagnitude =&gt; !referenceMagnitudeValue + !referenceMagnitudeError
         * !TransmissionPoint.spectralValue &mdash;-\ !transmissionValue + !spectralValue + !spectralErrorValue + unit
         * !TransmissionPoint.transmissionValue &mdash;/
         * !PhotometryFilter.spectralLocation =&gt; !SpectralLocation
<span style="white-space: pre;"> </span>However, Section 3.2.9.2: !TransmissionPoint.spectralValue and !transmissionValue describes !TransmissionPoint content as a !PhysicalQuantity with a specified !UType.

   * 
      * this text is unchanged from V1.0
      * the !UType given, is NOT in the !UType table of either version, but this could be considered a declaration of the !UType for a !PhysicalQuantity serialization
      * however, without !PhysicalQuantity, the elements involved can not be encapsulated in a single &lsquo;Quantity&rsquo; type and single !UType pointer 
         * !TransmissionPoint.unit
         * !TransmissionPoint.spectralValue
         * !TransmissionPoint.spectralErrorValue
<span style="white-space: pre;"> </span>Section 3.4.2: zeroPoint.referenceMagnitude.value

   * 
      * &ldquo;The reference magnitude is a dimensionless variable. It is modeled using a !PhysicalQuantity object type of appropriate precision (float,double).&rdquo; 
         * but it is not.. it is now just a &lsquo;real&rsquo;
-- IVOA.MarkCresitelloDittmar - 2022-05-17

---+++ [[IvoaOps][Operations]]

Mostly looks OK but a few comments.

   * Sec 1: _"This document proposes a standardization of a protocol to be used by Filter Profile Services."_ I don't see any standardization of a protocol here. The use case in Section 4.1 talks about _"Query[ing] the filter profile service"_ but doesn't explain how to do this. Am I missing something? As a DM document rather than a DAL one I don't say this document has to explain the protocol, but it shouldn't claim to do so if it doesn't. 
      * <em> agreed. Introduction section changed . Filter Profile service can implement this model for representing Filters. </em>-- IVOA.MireilleLouys - 2022-04-22
      * 
   * Sec 3.2.2: the example IVOID is "<code>ivo://svo/fps</code>". That IVOID doesn't exist, but "<code>ivo://svo.cab/fps</code>" does. It's only an example, so not necessarily required to reference a real service, but it seems like it might as well do so. 
      * _changed as suggested_ -- IVOA.MireilleLouys - 2022-04-22
   * Some of the document references could be updated, e.g. the citation to IVOA Identifiers is for v1.0 (2007) not v2.0 (2016). 
      * _updated , as well as some iother ivoa standard references . -- IVOA.MireilleLouys - 2022-04-22_
   * Sec 3.2.6: the ISOTime format specification should possibly defer to !DALI for consistency. If not, allowing the 'T' without a trailing time part is a bit strange. And there should possibly be some discussion of time zone. But really: is there ever going to be a case for sub-day resolution here? My uninformed guess is not, in which case it could be simplified to "YYYY-MM-DD". But if there's some good reason to stick with the text as written I don't insist on changes. 
      * _modified with reference to DALI -- IVOA.MireilleLouys - 2022-04-22_
   * Sec 3.4.4: the data type listed in the section heading is "enum", but "integer" is given for a similar usage in sec 3.1.2, and "integer" is also the term used for !ZeroPoint.type in the table in Appendix B. Make it consistent? 
      * _Done. -- IVOA.MireilleLouys - 2022-04-22_
   * Sec 3.8.2: capitalization and spelling of the terms VEGmag, ABmag, STmag is different from that presented in sec 3.8.1 (VEGAmag, ABMag, STMag) 
      * _Changed. -- IVOA.MireilleLouys - 2022-04-22_
   * Several of the items in the model are given with datatype "double", both in section 3 and Appendix B. In the examples in appendix C.1 and C.2 they are mostly written in the VOTables with =datatype="float"=. This sort of thing has caused problems with several standards in the past; I suggest making it clear somewhere that these model data types are allowed to use any floating point (or any numeric) type, e.g. by using a neutral term like "real" in the definitions. 
      * _Changed accordingly -- IVOA.MireilleLouys - 2022-04-22_
   * Appendix B: the tables are quite hard to read in the PDF; I know it's hard to format these things, but there is quite a bit of unused whitespace leading to overheight pages, so probably it could be improved. 
      * _Hard to reach a good compromise for lisibility with Latex . Not changed in the document. Model components in XML will be available on git repo -- IVOA.MireilleLouys - 2022-04-22_
   * Appendix C.1: indentation seems arbitrary for some of the listed XML, which makes it harder to read than necessary. Delete empty =unit=""= and =ucd=""= attributes unless there's some good reason for them. 
      * _Improved with the listing Latex environment._ _-- IVOA.MireilleLouys - 2022-04-22_
   * Appendix C.1: there are some errors in the VOTable. Use stilts [[http://www.star.bristol.ac.uk/~mbt/stilts/sun256/votlint.html][votlint]] to diagnose and fix. I can help on request 
      * _Done._ _-- IVOA.MireilleLouys - 2022-04-22_
-- IVOA.MarkTaylor - 2022-03-15

---+++ [[IvoaStdsDocsProc][Standards and Processes Committee]]

---

---++ TCG Vote :

If you have minor comments (typos) on the last version of the document please indicate it in the Comments column of the table and post them in the TCG comments section above with the date.

| Group | Yes | No | Abstain | Comments |
| TCG | | | | |
| Apps | | | | |
| DAL | | | | |
| DM | | | | |
| GWS | | | | |
| Registry | | | | |
| Semantics | | | | |
| DCP | | | | |
| KDIG | | | | |
| SSIG | | | | typos noted |
| Theory | | | | |
| TD | | | | |
| Ops | | | | |
| !RadioIG | | | | |
| !StdProc | | | | |

---

<br />&lt;!-- Set ALLOWTOPICRENAME =&lt;span&gt; IVOA.TWikiAdminGroup

&lt;!--<br /> * Set ALLOWTOPICRENAME = IVOA.TWikiAdminGroup
Attachments
Topic attachments
I	Attachment	History	Action	Size	Date	Who	Comment
png	BaseDataTypesdiagram.png	r1	manage	109.9 K	2022-05-20 - 13:25	MireilleLouys	Base Types classes and derived types ISOtime and UCD
png	PhotometryOverviewDiagram_jan22.png	r1	manage	156.9 K	2022-05-20 - 13:24	MireilleLouys	PHOTDM1.1 Overview diagram / Proposed Recommendation
html	Photv1-1_PR_20220520.html	r1	manage	112.7 K	2022-05-20 - 13:22	MireilleLouys	PHOT DM1.1 html documentation -generated from VODML XML
png	Photv1-1_update_jan22.png	r1	manage	94.9 K	2022-05-20 - 13:26	MireilleLouys	generated class diagram from VODML XML representation
Topic revision: r16 - 2022-05-20 - MireilleLouys
IVOA
Log in or Register
IVOA.net
Wiki Home
WebChanges
WebTopicList
WebStatistics
Twiki Meta & Help
IVOA
Know
Main
Sandbox
TWiki
TWiki intro
TWiki tutorial
User registration
Notify me
Working Groups
Interest Groups
Time Domain
Committees
Stds&Procs
www.ivoa.net
Documents
Events
Members
XML Schema