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

Extract prog gen to Origen Metal #204

Merged
merged 11 commits into from
Aug 2, 2024
Merged

Extract prog gen to Origen Metal #204

merged 11 commits into from
Aug 2, 2024

Conversation

ginty
Copy link
Member

@ginty ginty commented Aug 1, 2024

This PR moves all of the program generator functionality to Origen Metal, it doesn't add any additional test coverage, but the existing prog gen test (part of origen examples) works as before.

This de-couples the prog gen from Origen native concepts like the Origen app and its targets and adds a configuration API at origen_metal::PROG_GEN_CONFIG for hooking into Origen and other 3rd party frameworks.

Previously the SupportedTester class was shared between the patgen and the prog gen, but these have had to be separated and the prog gen now has its own one - it was too hard to share them without moving a lot of the patgen to OM.
I'm not sure if there is really any downside from this and don't see any immediate need to get back to a shared class.

New features:

  • A load path can be added via origen_metal::PROG_GEN_CONFIG::set_test_template_load_path to have Origen look for test method template definitions within the host application
  • When creating test suites and methods an error will now be raised by default if this is given any attribute arguments that don't correspond to an attribute of the given test. An allow_missing=True arg has been added to opt-into the old behavior of ignoring any attributes that are not recognized

Other updates:

  • Reinstates the origen save_ref command

@ginty ginty requested review from priyavadan, rlaj and coreyeng August 1, 2024 16:07
@ginty
Copy link
Member Author

ginty commented Aug 1, 2024

This file has been unchanged, however it is not active (not included) in the build, I think it can probably be deleted - https://github.com/Origen-SDK/o2/blob/e764d7ce9ff6ae2dad7e2f61d07c89c41733480e/rust/pyapi/src/prog_gen/tester.rs

@coreyeng
Copy link
Member

coreyeng commented Aug 2, 2024

Sounds good. I don't use much prog gen here yet, but I'm hoping to sooner or later. I haven't looked much at pattern generation in a long time, but if its tightly coupled to the app, we can look at separating those. My thoughts for much of that is "if the app is available, use it, but don't require an application just to work".

One question though, how does this relate to #203? Is that PR's content included in here?

@ginty
Copy link
Member Author

ginty commented Aug 2, 2024

Hi @coreyeng,

No, the #203 updates are not included here.

Once this is merged how do we do an origen_metal python release?

@coreyeng
Copy link
Member

coreyeng commented Aug 2, 2024

Okay. Quick note on #203 though, if you want to merge and we just make an issue as a future item to add that option, that's fine. Personally, I'd rathe we get stuff on master with a growing to-do list than sitting on branches. I've got all kinds of "TODOs" in my stuff and with so few of us, its always a matter of what's really needed vs. a nice-to-have.

For releasing, honestly, I don't remember exactly where I left off with the full action. I did get everything building for a manylinux2014 and windows build and I believe I had the full push working. I think I had the command on the rust CLI side to perform the release but there were some robustness and safety issues I had noted. I think for you I'd be fine, it was more for something like "I need to release origen but I have no idea about the Rust side or OM side, what checks do I need to safely allow this".

I believe it was in "origen publish" command in the CLI, which does some fancier things like lock the master branch to avoid a race condition while things are being built, bump the versions, update pyprojects, etc.

The actual action though is this one which you can set some options and run manually after the versions are updated and checked in:
image

This I did have working (most runs recently were canceled as this is just called from the publish command. I didn't actually want releases so I was canceling manually).

@ginty
Copy link
Member Author

ginty commented Aug 2, 2024

Ok, I'll give it a go and see if I can get it to release, thanks!

I'll leave #203 a bit longer, I do have an idea of how to resolve that and do plan to get back to it.

@ginty ginty merged commit 7af372e into master Aug 2, 2024
12 checks passed
@ginty ginty deleted the prog_gen branch August 2, 2024 14:48
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.

3 participants