-
Notifications
You must be signed in to change notification settings - Fork 6
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.
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
Specify operations and discovery #43
Conversation
f078386
to
2e6d747
Compare
Using a That said, should the ivoa.obs_radio schema include a mandatory obs_publisher_did column (and maybe also obs_id) to make sure there is at least something to join on? |
On Wed, Dec 13, 2023 at 03:08:27AM -0800, kettenis wrote:
That said, should the ivoa.obs_radio schema include a mandatory
obs_publisher_did column (and maybe also obs_id) to make sure there
is at least something to join on?
At first I thought I'd like the idea, but it has the big
disadvantage that without a lot of magic, this means SQL engines will
join on *at least* obs_publisher_did no matter what.
That is probably acceptable if we all agree that obs_publisher_did is
part of *a* primary key for both obs_radio and obscore (individual
providers can always have other primary keys that are computationally
cheaper and hence would probably be preferred by the query planner).
But I'm not sure everyone agrees on that.
Also, the case that there's nothing to join on at all is already
ruled out by "will yield per-dataset rows of obscore and
radio extension metadata" (or so I hope). Still, having *some*
reliable, stand-alone id column in obs_radio sounds like a reasonable
thing to want.
Perhaps we should have an issue on this and revisit it once there are
other implementations? For the record, for DaCHS it would be
trivial, because it will always join on obs_publisher_did (only).
|
ObsCoreExtensionForRadioData.tex
Outdated
are mostly covered by the ObsCore model, the lower level observational data | ||
(interferometric visibilities, single dish data in SDFITS, filterbank or whatever other specific formats) require additional description parameters. For interferometry, this was already exposed | ||
in 2010 when Anita Richards wrote a note called "Radio interferometry data in the VO" | ||
\footnote{https://wiki.ivoa.net/internal/IVOA/SiaInterface/Anita-InterferometryVO.pdf} which is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
66 to 77 OK
ObsCoreExtensionForRadioData.tex
Outdated
The scope of this Radio ObsCore extension in the IVOA is very close to ObsCore itself. | ||
Its goal is to add new specific features to the existing ObsCore metadata tables and expose | ||
them in the ObsTAP TAP\_SCHEMA. | ||
%The role of this Visibility ObsCore extension in the IVOA is very close to ObsCore itself. It's only a fine grain improvement of this specification. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why removing the diag ?
It seems that my recent comment on #49 is related to the changes proposed here. |
ObsCoreExtensionForRadioData.tex
Outdated
|
||
%We note that the interpretation of the spatial field of view as based on a mid/representative value for $\lambda$ should be better clarified in the ObsCore definition of this parameter. | ||
%We also point out that in the radio domain the field of view is an instantaneous concept, i.e. the instantaneous footprint (a circular primary beam in the case of mono-feed receivers, a more complex shape in the case of multi-feed/beamforming/PAFs), while in ObsCore it is defined as the approximate size of the region covered by the data product. | ||
%We also point out that in the radio domain the field of view is an instantaneous concept, i.e. the instantaneous footprint (a circular primary beam in the case of mono-feed receivers, a more complex shape in the case of multi-feed/beamforming/PAFs), while in ObsCore it is defined as the approximate size of the region covered by the data product. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
114-119 OK
ObsCoreExtensionForRadioData.tex
Outdated
Scanning parameters are applicable to both SD and interferometry. | ||
|
||
Pointing mode distinguishes targeted observations from fixed alt-azimuth or wobble ones. The ObsLocTAP specification \citep{2021ivoa.spec.0724S} defines the term tracking\_type for describing this as well as a vocabulary for these modes. We include the same term as an extension parameter here. | ||
|
||
|
||
\subsection{Auxiliary datasets useful for data quality estimation} | ||
|
||
Auxiliary datasets such as uv distribution map, dirty beam maps, frequency/amplitude plots, phase/amplitude plots are useful for astronomers to check data quality. | ||
Auxiliary datasets such as uv distribution map, dirty beam maps, frequency/amplitude plots, phase/amplitude plots are useful for astronomers to check data quality. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
242-397 OK
ObsCoreExtensionForRadioData.tex
Outdated
without having to know the exact foreign key(s) in use on any given | ||
service. This means that while one service can opt to join on | ||
\verb|obs_publisher_did| (we expect this to be a very common choice), | ||
other services may choose to join on, say, artificial keys. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion :
The ObsCore Extension for Radio data is accessed in a table with ObsCore within a TAP
\citep{2019ivoa.spec.0927D} service.\todo{I think we should require a
few UDFs on TAP services: ivo_interval_overlaps, ivo_specconv} At this
point, the name of this table is fixed to \verb|ivoa.obs_radio|.
However, in order to allow for later evolution -- which may allow
multiple compliant tables per service --, this name should not be used
for discovery (see sect.~\ref{sec:registry}).
A TAP service that has \verb|ivoa.obs_radio| must also have a table
compliant to any version 1 of ObsCore, i.e., a table
\verb|ivoa.obscore|; containing only the basic ObsCore attributes.
This table can be implemented as a simple view on the \verb|ivoa.obs_radio|
table
To ensure that all compliant services can execute the same queries,
all columns in table~\ref{tab:ExtensionAtt} must be present in such a
table, although any may be NULL. Additional columns may be present\todo{can we make rules such that such additional columns
will not interfere with later extensions?}.
The intention is that clients will write queries like
\begin{lstlisting}
SELECT [any obscore and obs_radio columns or expressions]
FROM
ivoa.obs_radio
WHERE
[constraints]
\end{lstlisting}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit hard to pick out your suggestion here...
You should be able to push into this branch -- so, can you just apply your proposed changes and merge? If I really do not like them, I can still do new PRs against the new main branch.
To produce a proper architecture diagram, please see https://ivoa.net/documents/Notes/IVOATexDoc/20230627/NOTE-ivoatexDoc-1.4-20230627.html#tth_sEc2.2.3 (the paragraph on the architecture diagram). Sorry for folding in the whitespace pruning in here, but, really, both are trivial changes.
Also, adding a section on how to register and discover obs_radio tables.
2e6d747
to
0d19356
Compare
So, I've rebased on the current main branch, which was nasty because of the whitespace changes. If you don't merge is soon-ish, please at least cherry-pick the whitespace commit, or following changes in main will keep being painful. But, really, my preferred way of continuing would be that you just apply whatever changes you see fit to this PR (you can push into this branch) and then merge. I'll complain if I see something I don't like... |
alternative option for implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with some text modified by myself. Initial text (radio table + registry) saved as commented text in the latex file
This replaces the implementation section with one that discusses ivoa.obs_radio, the way to join it with obscore, and then a section on registration and discovery.