Skip to content

Expose MicroSimulationInterface as public abstract base class for user subclassing #65#224

Merged
IshaanDesai merged 10 commits intoprecice:developfrom
AdityaGupta716:abstract-micro-sim-class
Mar 4, 2026
Merged

Expose MicroSimulationInterface as public abstract base class for user subclassing #65#224
IshaanDesai merged 10 commits intoprecice:developfrom
AdityaGupta716:abstract-micro-sim-class

Conversation

@AdityaGupta716
Copy link
Copy Markdown
Contributor

Closes #65

Exposes MicroSimulationInterface as a public abstract base class that users can inherit from when writing their micro simulations. This ensures users stick to the required API and get clear errors if they miss a method.

Changes:

  • Added full docstrings with examples to all abstract methods in MicroSimulationInterface
  • Exported MicroSimulationInterface from init.py so users can do from micro_manager import MicroSimulationInterface
  • Updated load_backend_class() to warn users with a DeprecationWarning if their class does not inherit from MicroSimulationInterface (will become a hard error in a future version)
  • Updated examples/python-dummy/micro_dummy.py to demonstrate the new inheritance pattern

@AdityaGupta716
Copy link
Copy Markdown
Contributor Author

AdityaGupta716 commented Feb 24, 2026

Hey @MakisH @IshaanDesai, plz review! Let me know if the deprecation warning approach works for you or if you'd prefer a hard error right away.

@AdityaGupta716 AdityaGupta716 changed the title Expose MicroSimulationInterface as public abstract base class for user subclassing Expose MicroSimulationInterface as public abstract base class for user subclassing #65 Feb 24, 2026
@MakisH MakisH added the GSoC Contributed in the context of the Google Summer of Code label Feb 25, 2026
@precice-bot
Copy link
Copy Markdown

This pull request has been mentioned on preCICE Forum on Discourse. There might be relevant details there:

https://precice.discourse.group/t/gsoc-2026-aditya-gupta/2773/1

@IshaanDesai IshaanDesai requested a review from Snapex2409 March 3, 2026 08:05
Copy link
Copy Markdown
Collaborator

@Snapex2409 Snapex2409 left a comment

Choose a reason for hiding this comment

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

Overall great idea and would be nice to have. Some minor changes requested, and please check regarding C++ bindings. Thank You.

Comment thread micro_manager/micro_simulation.py Outdated
Comment thread micro_manager/micro_simulation.py
Comment thread micro_manager/micro_simulation.py
Comment thread micro_manager/micro_simulation.py Outdated
@Snapex2409
Copy link
Copy Markdown
Collaborator

PS: run pre-commit run -a -v for example to get rid of the formatting issue in the CI.

@AdityaGupta716 AdityaGupta716 force-pushed the abstract-micro-sim-class branch from cc88ac8 to b975822 Compare March 3, 2026 13:28
@AdityaGupta716
Copy link
Copy Markdown
Contributor Author

@Snapex2409 hi , addressed all the review comments, Removed @AbstractMethod from initialize and output and made them optional with default pass implementations, added requires_initialize() and requires_output() for class-level override detection, and updated check_initialize() and check_output() to use these instead of hasattr. Also wrapped the issubclass check in a try/except for pybind11 compatibility,, added type hints throughout, and ran black to fix the pre-commit failure.
Couldn't test the C++ dummy directly since it needs pybind11 compiled locally, but the logic should handle it fine since non-inheriting classes fall back to the existing hasattr path.

Copy link
Copy Markdown
Collaborator

@Snapex2409 Snapex2409 left a comment

Choose a reason for hiding this comment

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

Overall thank you for your efforts. Nicely done. Please consider what I wrote in the comments. Let me know if I should clarify further.

Comment thread micro_manager/micro_simulation.py
Comment thread micro_manager/micro_simulation.py Outdated
Comment thread micro_manager/micro_simulation.py Outdated
@AdityaGupta716
Copy link
Copy Markdown
Contributor Author

@Snapex2409 hi done with the changes , Wrapped non-conforming classes in a dynamic adapter in load_backend_class so it always returns a proper MicroSimulationInterface subclass. The adapter delegates everything via getattr and only adds initialize/output overrides if the original class actually has them — so requires_initialize()/requires_output() work correctly for all cases including pybind11. check_initialize and check_output are now clean with no guards.
Also caught a bug while implementing — the wrapper chain (MicroSimulationWrapper, MicroSimulationLocal, MicroSimulationRemote) was unconditionally overriding initialize and output, which made requires_initialize()/requires_output() always return True. Fixed that too.
Still can't test the C++ dummy locally, but the logic should handle it correctly since the TypeError from issubclass triggers the wrap path and that test fail seems unrelated

Copy link
Copy Markdown
Collaborator

@Snapex2409 Snapex2409 left a comment

Choose a reason for hiding this comment

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

Looks very good. Thank you 👍

Could you determine why the test fails?

The suggested changes are just to have the same docstring style as in other files.

Comment thread micro_manager/micro_simulation.py Outdated
Comment thread micro_manager/micro_simulation.py Outdated
Comment thread micro_manager/micro_simulation.py Outdated
Comment thread micro_manager/micro_simulation.py Outdated
Comment thread micro_manager/micro_simulation.py Outdated
Comment thread micro_manager/micro_simulation.py Outdated
Comment thread micro_manager/micro_simulation.py Outdated
Comment thread micro_manager/micro_simulation.py Outdated
Comment thread micro_manager/micro_simulation.py Outdated
@AdityaGupta716
Copy link
Copy Markdown
Contributor Author

@Snapex2409 Hi , applied all the docstring suggestions
Looked into the CI failure too — reproduced it on upstream/develop directly with no changes from this PR, so it's pre-existing. The root cause is in test_set_state itself: it calls del sim mid-test which triggers cleanup on the MicroSimulationRemote object, then immediately tries to reuse the same worker connection to send a GetStateTask. The workers have already lost state at that point, so recv() hits an EOFError. It's a resource management issue in the test unrelated to our changes.
Happy to fix it in this PR if that works for you

@Snapex2409
Copy link
Copy Markdown
Collaborator

@AdityaGupta716 I think you did not check out develop while testing, but that's ok. Issue is resolved now.
Thank you for your help.

Apart from that: As you already mentioned, that is not clean resource management. But I am already addressing that in another PR. So, no need to deal with that here.

@AdityaGupta716
Copy link
Copy Markdown
Contributor Author

@Snapex2409 i Ran test_set_state on upstream/develop locally it gets stuck in setUp at spawn_local_workers, hanging on server.accept() waiting for a worker to connect. prterun can't find the python executable so the worker never spawns and the socket just hangs. Had to kill it manually

Screenshot 2026-03-04 at 3 12 04 AM

@Snapex2409
Copy link
Copy Markdown
Collaborator

@AdityaGupta716 Looks like your mpi is not able to find to find python. What python environment are you using venv, conda or other? Is it perhaps not in PATH?

@AdityaGupta716
Copy link
Copy Markdown
Contributor Author

@Snapex2409 ahh got it my bad was using venv and the path is python3 not python , got it

@IshaanDesai IshaanDesai merged commit 1a97da9 into precice:develop Mar 4, 2026
8 checks passed
@IshaanDesai IshaanDesai mentioned this pull request Apr 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

GSoC Contributed in the context of the Google Summer of Code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide a micro simulation abstract class which users can inherit/create a subclass

5 participants