Conversation
robyww
force-pushed
the
FIREFLY-1931-asdf
branch
from
b1c8573 to
586bcd7
Compare
- alpha - only data under "roman" node - uses roman layout - support images - support tables
robyww
force-pushed
the
FIREFLY-1931-asdf
branch
from
586bcd7 to
bce1dde
Compare
|
Exciting to see this and test it. Thanks! |
|
@ejoliet I can only tell you what is in the asdf file. Here is the yaml part of one of the asdf files we are using. I see so many different versions in it! |
loitly
left a comment
loitly
left a comment
There was a problem hiding this comment.
I tested this with a sample file and it worked as expected. The code looks clean. Implementing inline table ingestion would be a nice enhancement if we decide to pursue it. Otherwise, this looks good to go.
| static FileInfo ingestAsdfTable(TableServerRequest req, DbAdapter dbAdapter, String source, int tableIndex) throws IOException, DataAccessException { | ||
| var table = ASDFUtil.convertAsdfAstroPyTableToDataGroup(new File(source), req, tableIndex); | ||
| return ingestTable(dbAdapter, table, false); //logic is already in the reader | ||
| } |
There was a problem hiding this comment.
This works, but it requires loading the entire table into memory during database ingestion, which demands higher deployment memory limit when processing large files.
We have a TableParseHandler.DbIngest option that supports inline ingestion and uses minimal memory, regardless of table size.
src/firefly/java/edu/caltech/ipac/firefly/server/visualize/ImagePlotBuilder.java
Show resolved
Hide resolved
src/firefly/java/edu/caltech/ipac/firefly/server/visualize/ImagePlotBuilder.java
Show resolved
Hide resolved
| int startingTableIdx= asdfInfo.imageNameList.size(); | ||
| if (tableIdx>=startingTableIdx+asdfInfo.tables.size()) throw new IOException("table index exceeds number of tables"); | ||
| String tName= asdfInfo.tableNames().get(tableIdx-startingTableIdx); |
There was a problem hiding this comment.
So tableIdx refers to the index within the ASDF file itself, not the index of the tables, right?
Also, are images always located at the beginning of the file, with tables at the end? If not, I don't think this logic will work.
There was a problem hiding this comment.
Yes, (at least for now) the processing make all the images it finds to be first then then all the tables. It is not necessarily about the structure of the file just an arbitrary way to organize it.
This is basically flattening out a very deep structure into something more simple for the user.
There was a problem hiding this comment.
It would be frustrating for users who understand the file structure to have to work with a manufactured version. I am pretty sure someone will raise the issue.
There was a problem hiding this comment.
I don't think there is a solution for that right now. For asdf, an image or a table does not have a numbered location. It is just at some arbitrary place. So our numbering scheme it just to help the user get a sence of it.
In my recent meeting I did talk to the ASDF folks about adding a table of contents to the file format. If that ever happens we will use that.
Also remember this is just an first step at this. I am sure we will iterate on it several times.
ejoliet
left a comment
ejoliet
left a comment
There was a problem hiding this comment.
I played with one of the file in the box.com shared - those are Level-2 images - i can visualize all the extensions and check tables as well. Congrats!
As i mentioned in a separate discussion, there is much more complex products in ASDF that might requires more logic to extract data that originally image extractor could do.
The other twist is making sure to highlight the compatibility with datamodel version used, which uniquely identified the content. Typically this is the version of the software, roman_datamodels and the RAD.
One thing you could always show even if you can't extract data is to show the header, which is very useful and is YAML (ASCII) so you don't need any decoder.
For images based ASDF, i think it's already a win!! I will share the tool once is released so people can start using it.
3 participants
Firefly-1931: asdf file alpha support
romannode, only uses roman layoutReviewer notes.
ASDFUtil.java, the other file have only have small change to recognize the ASDF files.Testing
Boxabove, I am not sure others would work but we should try them.