Skip to content

Conversation

@cdcapano
Copy link
Contributor

This adds the ability for users to define their own models that can be used in pycbc inference, without needing to modify the source code. Borrows the same techniques as used for the plugin waveforms.

Copy link
Member

@ahnitz ahnitz left a comment

Choose a reason for hiding this comment

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

Looks right.

@ahnitz
Copy link
Member

ahnitz commented May 28, 2022

@cdcapano Proposed change to your PR to fix the circular import issue cdcapano#5

@cdcapano
Copy link
Contributor Author

I was having trouble with circular imports. Basic problem was custom model classes need to inherit from BaseModel, but this setup a circular import since the plugins are resolved when the models module is initialized. To fix, I've delayed the plugin resolving to when the models are first accessed, rather than when they're imported. I did this by replacing the inference.models.models dict with a function, inference.models.get_models(). This will break backward compatibility for anyone that previously accessed the models dict directly (I think this is done in a couple of the tutorials). It doesn't affect the pycbc_inference executable though, so should probably be ok.

@cdcapano
Copy link
Contributor Author

cdcapano commented Jun 2, 2022

@ahnitz To avoid breaking backward compatibility, I decided to duck type the models dictionary instead so that it automatically adds the plugins the first time its accessed. I think I covered all possible ways it could be accessed. I left in the new functions, with the idea that these should be used anyway. If the checks pass I'll merge, unless you object.

I'll add documentation and a tutorial in another PR.

@ahnitz
Copy link
Member

ahnitz commented Jun 2, 2022

@cdcapano Ok, sounds good.

@cdcapano cdcapano enabled auto-merge (squash) June 2, 2022 12:54
@cdcapano cdcapano merged commit f142f1c into gwastro:master Jun 2, 2022
connor-mcisaac pushed a commit to connor-mcisaac/pycbc that referenced this pull request Oct 12, 2022
* add ability to load plugin models

* try to avoid circular imports

* fix circular import by using functions to retrieve models

* use get_models for building the models table

* duck type models dict

* code climate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants