Photometry Data Model 1.1 Proposed Recommendation: Request for CommentsIntroductionPHOTDM 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 :
The latest version of the model and supporting docs:
Implementation Requirements
Comments from the IVOA Community during RFC/TCG review period: 2022-03-07 - 2022-04-22The 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 documentComments by MarkusDemleitnerDear 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 -- 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. -- JesusSalgado - 2022-04-11 (2) As a general request, it would be great if lines could be reflowed to <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) -- 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 $(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 -- 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 -- 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 -- 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 -- 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 -- 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) -- 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 -- 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 -- 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:
(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 -- 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’s time series serialisation so, in order to prevent couplings, I have updated the title of the section to singular -- 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 -- 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 -- 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 -- 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 -- 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 -- 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 -- 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. So, when it says: could include, it tries to say: For catalogues that include photometric measurements… And when it says: will be as follows: it tries to say: The workflow to make use of this capability is the following: I have modified the text in this line. Just to mention that this capability was integrated into ViZieR in the server side and in VOSpec in the client side as reference implementations. -- JesusSalgado - 2022-04-11 (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! -- JesusSalgado - 2022-04-11 -- 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 ChairApplications Working GroupData Access Layer Working GroupData Model Working GroupGrid & Web ServicesRegistry Working GroupSemantics 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.
Education Interest GroupKnowledge Discovery Interest GroupSolar System Interest GroupI'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.
| |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Deleted: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
< < | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
A simplification of the sentence has been added following the spirit of the original text
-- JesusSalgado - 2022-04-25
| |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Deleted: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
< < | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Done and reference updated
-- JesusSalgado - 2022-04-25
| |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Deleted: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
< < | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Typos corrected. Thanks for the review Anne
-- JesusSalgado - 2022-04-25
-- AnneRaugh - 2022-04-20
Theory Interest GroupTime Domain Interest Group | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Added: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
> > |
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:
The UTypes for the endpoints are unchanged, so I suppose the thought is that this is sufficient for backward compatibility..
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
OperationsMostly looks OK but a few comments.
Standards and Processes CommitteeTCG 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.
<!-- Set ALLOWTOPICRENAME =<span> TWikiAdminGroup <!-- * Set ALLOWTOPICRENAME = TWikiAdminGroup |