Skip to content
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

TOML SerializationFormat fails for GdsLibrary #33

Open
dan-fritchman opened this issue Dec 15, 2022 · 3 comments
Open

TOML SerializationFormat fails for GdsLibrary #33

dan-fritchman opened this issue Dec 15, 2022 · 3 comments

Comments

@dan-fritchman
Copy link
Owner

dan-fritchman commented Dec 15, 2022

This:

/// Create an empty library with known dates
fn empty_lib() -> GdsLibrary {
    // Create an empty library
    let mut lib = GdsLibrary::new("empty");
    // Set its dates to some known value, so we can check it round-trips
    lib.dates = test_dates();
    // And return it for other test
    lib 
}
#[test]
fn empty_lib_to_toml() -> GdsResult<()> {
    let lib = empty_lib();
    Toml.save(&lib, &resource("empty.gds.toml")).expect("save failed");
    Ok(())
}

Fails with:

failures:

---- tests::empty_lib_to_toml stdout ----
thread 'tests::empty_lib_to_toml' panicked at 'save failed: Error(UnsupportedType)', gds21/src/tests.rs:214:50
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Notes:

  • Yaml & Json both work
  • Yaml, Json, and Toml all work for lef21::LefLibrary

Unclear at this point what the UnsupportedType is.

Unclear when this was injected, as we had generally only been testing one format of serialization (GDS to JSON, LEF to YAML, nothing to TOML). And it wasn't clear that there are value-types supported in YAML & JSON that aren't in TOML(?). Apparently GdsLibrary has one or more of them.

@dan-fritchman
Copy link
Owner Author

dan-fritchman commented Dec 19, 2022

Rooting around GdsLibrary, without testing this at all, I have a bet, and that bet is, it's this date & time stuff:

/// # Gds Date & Time
///
/// In typical cases, a wrapper around a [`NaiveDateTime`] with custom serialization.
/// For existing GDSII files with invalid dates, the raw twelve bytes are stored instead.
///
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)]
pub enum GdsDateTime {
    /// Valid Date & Time
    DateTime(NaiveDateTime),
    /// Raw Bytes as stored in GDSII
    Bytes([i16; 6]),
}

@dan-fritchman
Copy link
Owner Author

Bet lost.
Datetimes are not a part of GdsLibrary as of acd6ac7, and this still fails.

Errors have upgraded to:

---- gds_serialization::tests::golden_toml stdout ----
GdsStats { libraries: 1, structs: 1, boundaries: 144, paths: 0, struct_refs: 0, array_refs: 0, text_elems: 10, nodes: 0, boxes: 0 }
thread 'gds_serialization::tests::golden_toml' panicked at 'Could not save output file: values must be emitted before tables', layout21converters/src/gds_serialization.rs:56:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

We've got an open question with the toml authors as to what should work here. I think our stuff should? But clearly it doesn't.

@dan-fritchman dan-fritchman changed the title TOML SerialzationFormat fails for GdsLibrary TOML SerializationFormat fails for GdsLibrary Dec 20, 2022
@dan-fritchman
Copy link
Owner Author

toml-rs/toml#396 was recently closed, and updated in TOML v0.6.
Commit 812ee2d includes v0.6. It makes the empty GDS to TOML work. But the sample in layout21converters/ continues to fail, now (and after peeling off our own errors) with:

     Running `gds2toml -i resources/sky130_fd_sc_hd__dfxtp_1.gds -o test.toml`
thread 'main' panicked at 'Could not save output file: unsupported GdsElement type', layout21converters/src/gds_serialization.rs:42:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

This is hitting TOML's unsupported_type function and error-variant.

Much of the guesswork (by me) thus far on the TOML issue is that this boils down to TOML's "values must come before tables" rule. It seems (from its commentary) that the Rust TOML package should paper over this. But then again maybe not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant