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

Gromet Wiring Detector PR #732

Merged
merged 9 commits into from
Dec 22, 2023
Merged

Gromet Wiring Detector PR #732

merged 9 commits into from
Dec 22, 2023

Conversation

titomeister
Copy link
Contributor

@titomeister titomeister commented Dec 15, 2023

This PR adds a new script skema/program_analysis/gromet_wire_diagnosis.py that can be used to do some simple analysis and error detecting in the wires of GroMEt FNs. It currently checks the ports of all types of wires, and detects whether the ports are out of bounds (in either negative or positive indices) within their respective port tables. It also attempts to find the most relevant SourceCodeReference metadata that is associated with the wires and displays the line number information contained within it.

Summary of Changes

  • Adds skema/program_analysis/gromet_wire_diagnosis.py script
  • Modifies skema/program_analysis/JSON2GroMEt/json2gromet.py script so that it can ingest newer GroMEt JSON that uses the updated Gromet metadata fields.
  • Fixes a small issue with the incorrect SourceCodeReference metadata type being used in skema/program_analysis/CAST2FN/ann_cast/to_gromet_pass.py
  • Adds a test script skema/program_analysis/tests/test_wiring_diagnosis.py that can test the consistency of the individual wire checker utility without needing a GroMEt JSON.

Potential Next steps

  • Need to determine more things that can be easily analyzed in a GroMEt JSON
  • Come up with a more robust way of determining what line numbers go with the wires.
  • Determine what to do with the Metadata fields that don't have an "is_metadatum" field attached to them. (NOTE: A solution to this has been currently proposed.)

Resolves #697

@vincentraymond-ua
Copy link
Contributor

@titomeister - You mentioned Metadata types that don't have an is_metadatum field. These should be handled by these lines

# If there is not a mapping to an object, we will check the fields to see if they match an existing class in the data-model.
# For example: (id, box, metadata) would map to GrometPort
obj_fields = tuple(obj.keys())
if obj_fields in gromet_field_map:
return gromet_field_map[obj_fields]()
for gromet_name, gromet_object in inspect.getmembers(
sys.modules[__name__], inspect.isclass
):
found = True
for field, value in obj.items():
if not hasattr(gromet_object, field):
found = False
break
if found:
gromet_field_map[obj_fields] = gromet_object
return gromet_object()

. If this no longer works though let me know and we can figure something else out.

@titomeister
Copy link
Contributor Author

titomeister commented Dec 18, 2023

@vincentraymond-ua So the issue I was running into is that the code would break on the if-else statement that is directly before this chunk of code.

if "gromet_type" in obj and ("is_metadatum" not in obj or obj["is_metadatum"] != True):
return gromet_fn_map[obj["gromet_type"]]()
elif obj["is_metadatum"]:
return gromet_metadata_map[obj["gromet_type"]]()

Currently there is temporary handling for the metadata types that don't have an is_metadatum field, but I know there's something better. Whether that involves just adding the is_metadatum field to those metadata types or something else.

The current handling can be seen here

if "is_metadatum" in instance.attribute_map and instance.is_metadatum:
gromet_metadata_map[metadata_name] = metadata_object
else:
gromet_fn_map[metadata_name] = metadata_object

Basically, the metadata types that don't have an is_metadatum field get added to the gromet map on line 38. I figured that is okay for the moment, since I believe those metadata types don't appear in the GroMEt JSON at all.

@titomeister titomeister marked this pull request as draft December 19, 2023 16:55
@titomeister titomeister marked this pull request as ready for review December 20, 2023 19:29
Copy link
Contributor

@vincentraymond-ua vincentraymond-ua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@titomeister - Looks good! Eventually it would be nice to integrate with Justin's fn_preprocessor so that this shows up in the model coverage reports as well.

@titomeister titomeister merged commit 97f7e50 into main Dec 22, 2023
7 checks passed
@titomeister titomeister deleted the ferra/gromet_wiring_detector branch December 22, 2023 05:14
github-actions bot added a commit that referenced this pull request Dec 22, 2023
This PR adds a new script
`skema/program_analysis/gromet_wire_diagnosis.py` that can be used to do
some simple analysis and error detecting in the wires of GroMEt FNs. It
currently checks the ports of all types of wires, and detects whether
the ports are out of bounds (in either negative or positive indices)
within their respective port tables. It also attempts to find the most
relevant SourceCodeReference metadata that is associated with the wires
and displays the line number information contained within it.

## Summary of Changes
- Adds `skema/program_analysis/gromet_wire_diagnosis.py` script
- Modifies `skema/program_analysis/JSON2GroMEt/json2gromet.py` script so
that it can ingest newer GroMEt JSON that uses the updated Gromet
metadata fields.
- Fixes a small issue with the incorrect SourceCodeReference metadata
type being used in
`skema/program_analysis/CAST2FN/ann_cast/to_gromet_pass.py`
- Adds a test script
`skema/program_analysis/tests/test_wiring_diagnosis.py` that can test
the consistency of the individual wire checker utility without needing a
GroMEt JSON.

### Potential Next steps
- Need to determine more things that can be easily analyzed in a GroMEt
JSON
- Come up with a more robust way of determining what line numbers go
with the wires.
- Determine what to do with the Metadata fields that don't have an
"is_metadatum" field attached to them. (NOTE: A solution to this has
been currently proposed.)

Resolves #697

---------

Co-authored-by: Vincent Raymond <[email protected]> 97f7e50
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

Successfully merging this pull request may close these issues.

[code2FN] Script for GroMEt Wiring Diagnosis
2 participants