ml4ai / automates-v2

AutoMATES: Automated Model Assembly from Text, Equations and Software
2 stars 3 forks source link

Code generation and need for discriminator #36

Open kwalcock opened 1 year ago

kwalcock commented 1 year ago

Java/Scala code generation doesn't work from files like gromet_FN_v0.1.4.yaml and gromet_metadata_v0.1.4.yaml for lack of a discriminator in superclasses. For example,

    GrometObject:
      description: Generic base class for any GroMEt object
      type: object
      properties:
        metadata:
          description: Index (integer) into the metadata_collection table in GrometFNModule.
          type: integer
# Lines like the ones below need to be added     
        gromet_object_type: # added
          type: string #
      discriminator: # added
        propertyName: gromet_object_type

When subclass objects are written out, a field should then appear as such:

{
    "gromet_object_type": "GrometFNModule",
    "schema": "FN",
    "schema_version": "0.1.4",

This allows the generated Java/Scala code to figure out how to deserialize the object.

I don't (yet) know how the Python code is working without this, but will probably notice soon. I am hand editing the files like cond1--Gromet-FN-auto.json to add this information to make sure the updates to the yaml file that I'm about to suggest work.

Perhaps this issue has been encountered before, so I'm filing this slightly prematurely in case there's more I should know before continuing.

kwalcock commented 1 year ago

@cl4yton, are you the one to ask about this?

cl4yton commented 1 year ago

Thanks @kwalcock for pointing this out, and yes, I'm among the folks to bug about this. I think this may have gone unnoticed by us so far. I'm adding @vincentraymond-ua and @titomeister to this thread.

@vincentraymond-ua: Can you take a look at this, given you familiarity with the importer?

cl4yton commented 1 year ago

Commenting again b/c I just realized I had not added @vincentraymond-ua to this project until just now...

vincentraymond-ua commented 1 year ago

In the case of the Python importer, we can work around this since there is no ambiguity in the Gromet about what type of object we are dealing with. For example, the entries in "bf" will always be a GrometBoxFunction, the entries in "bc" will always be a GrometBoxConditional, and the entires in "bl" will always be a GrometBoxLoop.

So, we just use that knowledge to create the correct type. I can't think of or recall an example of a field where we can have more than one type of GrometObject, but I could be incorrect.

@titomeister @cl4yton - What do you think?

vincentraymond-ua commented 1 year ago

Also, I do see value in adding a discriminator for Gromet Objects, and we already have something similar for the superclass Metadata (metadata_type)

cl4yton commented 1 year ago

Hi @kwalcock @vincentraymond-ua @titomeister : I could use some discussion on this as I think I might not understand the issue. I just create an ASKEM PA meeting for tomorrow (Tito and Vincent: We can review progress on our task list), but also invited @kwalcock (if you are available) to join at the beginning to see if we can work out the issue.

kwalcock commented 1 year ago

Yes, I can be at that meeting. I was surprised that the client generated for Java would not not process the files, even though it looks perfectly feasible. FWIW, the code I'm using is basically

new io.swagger.client.JSON().deserialize(text, classOf[GrometFNModule])

where text comes from files like gromet_FN_v0.1.4.yaml. It seems like the Java code generator is assuming that these types will be used polymorphically so that it will by necessity need to have a hint about which subclass is being encountered. It might also have to do with the gson being used under the hood. Whatever the reason, it seems like adding the discriminator works. In not too long I could know for certain.

I realize we're mostly using Python mostly, but guess that some other group might want to use the definitions in a different way. It looks like it wouldn't have worked out of the box.

kwalcock commented 1 year ago

It looks like the program to work around some of the problems with having the model in two separate files is this:

https://github.com/ml4ai/automates/blob/master/scripts/swagger/codegen_swagger_models.py

I'm thinking that I can instead read the two yaml files, combine the component/schemas path of each and generate a single set of files. That's going to make life easier for me while editing the files, so I'll try it.

kwalcock commented 1 year ago

Here are some notes about code generation:

I wrote programs to generate code via either of the libraries

    "io.swagger.codegen.v3" % "swagger-codegen-cli"   % "3.0.35",
    "org.openapitools"      % "openapi-generator-cli" % "6.2.0",

from https://github.com/swagger-api/swagger-codegen and https://github.com/OpenAPITools/openapi-generator and then tried to read in the json documents that had already produced. For the swagger library, neither the code generated for java nor scala was able to read in the files, even with a substantial amount of rewriting the generated code. For the openapitools, none of generators for scalatra, scala-akka, scala-finch, scala-logom-server, scala-play-server, scala-sttp, or scalaz produced hopeful results. They weren't exactly hopeless, but it was taking significantly longer to figure out what was wrong with them or how to use them than it would take to translate the spec by hand, at least if the conversion only needed to be done once. I did do that translation and it reads in all the files. It's right now in https://github.com/kwalcock/skema and specifically at https://github.com/kwalcock/skema/tree/main/gromet/model-scala-v0.1.4/src/main/scala/org/ml4ai/skema/gromet/model/scala/v0_1_4.

I would not say "To generate Java classes rather, change the -l python to -l java." Code may be generated, but it isn't usable.