nemocrys / pyelmer

A python interface to Elmer.
GNU General Public License v3.0
56 stars 18 forks source link

Added Components #33

Closed Aerosmite closed 1 year ago

Aerosmite commented 1 year ago

Added simple component class and integrated it to Simulation.

arvedes commented 1 year ago

Thank you for this nice contribution! Can you add a few tests for the new features to pyelmer/tests/test_elmer.py?

Aerosmite commented 1 year ago

Unfortunately I can't share mine as it is a company project. You can try to generate one of the many example of magnetic simulation in elmer test examples, which often use components.

Also made a small mistake in the Component class, you need to have a name attribute otherwise you get a warning:

def get_data(self):
    """Generate dictionary with data for sif-file."""
    d = {
        "Name": self.name,
        f"Master Bodies({len(self.geo_ids)})": " ".join([str(x) for x in self.geo_ids] )
    }
    d.update(self.data)
    return d
arvedes commented 1 year ago

Thanks for the fix! I don't know if we're talking about the same thing. I meant the pyelmer tests, not a complete Elmer simulation. The tests are run with pytest and check the main classes and functions.

It would be nice to have a test for the components, too, e.g. a test_component function similar to test_body and test_duplicate_body.

arvedes commented 1 year ago

I just added the tests. I don't have any experience with Components yet, but in the Elmer models manual, it seems to me as if the parameter Master Bodies in the sif file is supposed to refer to a Body also defined in the sif. The way it's implemented now it is supposed to refer to a part of the geometry. Can you clarify that for me?

Aerosmite commented 1 year ago

I don’t have access to my computer currently so I can’t be 100% sure, but according to the Elmer doc and because my geometry ids are the same than my body ids in my simulations I think you’re right. Sorry I didn’t see your previous message.

Thanks a lot for your tool anyway! At some point I would have been bored and coded the same thing than you so it really helped me save time!

arvedes commented 1 year ago

You're welcome! I updated it so that the component class now has the parameters master_bodies and master_boundaries instead of geo_ids.

What do you think, is it ok like that? Can I merge it?

Aerosmite commented 1 year ago

I can’t test it now but it looks perfect!

arvedes commented 1 year ago

Ok, I merged it and created a new release. Tanks again!