fusion-energy / paramak

Create parametric 3D fusion reactor CAD and neutronics models
https://fusion-energy.github.io/paramak
MIT License
67 stars 21 forks source link

preparing to move to cadquery 2.2 #153

Open shimwell opened 2 years ago

shimwell commented 2 years ago

Currently the paramak requires cadquery 2.1

There is a really nice importBrep feature in the upcoming CadQuery 2.2 and this feature is essential for the use of Paramak geometry in the neutronics workflow. So Paramak will have to be upgraded to work with CadQuery 2.2 once it is released.

Currently the importBrep feature is available on the CadQuery master branch.

There are some API changes that appear to be in CadQuery 2.2 (currently in master branch)

This issue is aiming to note the changes so that they can be accounted for in the release of the Paramak that makes use of CadQuery 2.2

shimwell commented 2 years ago

wire.extrude() previously made use of the distance argument, this has been changed to until

ZedThree commented 2 years ago

Here's a possible solution:

from packaging import version

if version.parse(cq.__version__) <= version.parse("2.1"):
    # Keep reference to current implementation
    old_extrude = cq.Workplane.extrude
    # Wrapper that converts argument names
    def extrude(self, until, combine, clean, both, taper):
        return old_extrude(
            self, distance=until, combine=combine, clean=clean, both=both, taper=taper
        )
    # Monkey-patch class
    cq.Workplane.extrude = extrude

This sort of backport or polyfill lets you upgrade your code to use the latest version while still maintaining compatibility with previous versions.

shimwell commented 2 years ago

Nice, I am learning lots of ways to use __version__ from you both

LiamPattinson commented 2 years ago

I don't think the proposed solution will work until the release of CadQuery 2.2. Version 2.2 can't be installed from conda yet, and within the master branch the version is still reported as 2.1.

I have a slight variation of this in mind, in which we try calling extrude with 'distance', catch a TypeError if it fails, and then perform the patch.

LiamPattinson commented 2 years ago

I've had some luck with this variation:

def patch_workplane():
     """Going from CadQuery 2.1 to 2.2, the 'distance' arg to extrude was renamed 'until'.
     This patch ensures that either version works fine using 'until'.
     """
     if 'distance' in Workplane.extrude.__code__.co_varnames:
         extrude_func = Workplane.extrude
         def extrude( *args, **kwargs):
             if "until" in kwargs.keys():
                 kwargs["distance"] = kwargs.pop("until")
             return extrude_func( *args, **kwargs)
         Workplane.extrude = extrude

Since we can't rely on the version number, we simply check whether 'distance' features as an argname for Workplane.extrude. This patch function needs to be called at the top of every file that uses extrude as far as I know, but it may be possible to instead call it at the module level.

According to my testing, it works for both 2.1 and 'master', although using CadQuery master leads to 4 unit tests under tests/test_parametric_components failing:

I'm not really sure what any of that means, and it may be due to other CadQuery changes since 2.1. Shall I set up a pull request?

shimwell commented 2 years ago

I don't think the proposed solution will work until the release of CadQuery 2.2. Version 2.2 can't be installed from conda yet, and within the master branch the version is still reported as 2.1.

I have a slight variation of this in mind, in which we try calling extrude with 'distance', catch a TypeError if it fails, and then perform the patch.

I have been using a version of try except elsewhere that doesn't check version numbers

https://github.com/fusion-energy/paramak/blob/96a0cbfcbbe73a044bc5ce0faa3292f4ae027f52/paramak/parametric_shapes/extruded_mixed_shape.py#L86-L89

shimwell commented 2 years ago

Yes please on the PR for patch_workplane

The failing unit tests are due to another API change in CQ master that i can raise an issue for. But don't worry about the failing tests, I can raise issue and raise a PR to sort that

shimwell commented 2 years ago

@LiamPattinson spotted an API change in CQ master compared with CQ 2.1

When accessing the solids in compound solids the API has changed a little bit.

I believe the isinstance(self.solid, Compound): method used no longer works

The new method appears to be

.solid.val().Solids() which returns a list of solids and I've been using it here https://github.com/fusion-energy/paramak/blob/96a0cbfcbbe73a044bc5ce0faa3292f4ae027f52/paramak/reactor.py#L274

The old method is .solid.Solids()

So I believe these parts will need updating https://github.com/fusion-energy/paramak/blob/96a0cbfcbbe73a044bc5ce0faa3292f4ae027f52/paramak/shape.py#L313-L316

https://github.com/fusion-energy/paramak/blob/96a0cbfcbbe73a044bc5ce0faa3292f4ae027f52/paramak/shape.py#L320-L330

https://github.com/fusion-energy/paramak/blob/96a0cbfcbbe73a044bc5ce0faa3292f4ae027f52/paramak/shape.py#L1393-L1406

This can get a bit messy trying to accommodate CQ2.1 and CQ master and we should be aware that CQ master can change prior to CQ 2.2 release. So I suggest this is done on an new PR

We might want new tests to double check these functions still work. For reference a compound shape can be made like this


blanket = paramak.BlanketConstantThicknessArcH(
    inner_mid_point=(500, 0),
    inner_upper_point=(400, 300),
    inner_lower_point=(400, -300),
    thickness= 100,
    rotation_angle=40,
    azimuth_placement_angle=[0, 45, 90, 135, 180, 225, 270, 315],
    name='blanket'
)
print(blanket.area)
print(blanket.areas)
print(blanket.volume(split_compounds=True)