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

Lens #21

Merged
merged 17 commits into from
Oct 1, 2018
Merged

Lens #21

merged 17 commits into from
Oct 1, 2018

Conversation

mlin865
Copy link
Contributor

@mlin865 mlin865 commented Sep 30, 2018

No description provided.

Copy link
Member

@rchristie rchristie left a comment

Choose a reason for hiding this comment

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

Looks good, but some improvements.

cls.generateBaseMesh(baseRegion, options)

meshrefinement = MeshRefinement(baseRegion, region)
meshrefinement.refineAllElementsCubeStandard3d(refineElementsCountAround, refineElementsCountUp, refineElementsCountRadial)
Copy link
Member

Choose a reason for hiding this comment

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

Add a newline to the end.

options['Anterior radius of curvature'] = options['Axial thickness']*0.5

@classmethod
def sphereToLens(cls, region, options):
Copy link
Member

Choose a reason for hiding this comment

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

Pass actual parameters to this function, not options dict. I.e.. coordinates, radiusSphere, radiusAnt, radiusPos, lensThickness, sphericalRadiusFraction etc. Describe each argument using :param NAME: description.

Can get fieldmodule from coordinates field, so no need to pass:
fm = coordinates.getFieldmodule()

Function and description should state that it defines a field giving positions on a spherical lens as functions of positions in the sphere.

I would make this a standalone function e.g.:
def getSphereToLensCoordinates(args):

"""
fm = region.getFieldmodule()
fm.beginChange()
cache = fm.createFieldcache()
Copy link
Member

Choose a reason for hiding this comment

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

This is unused. Remove.


class MeshType_3d_lens1:
'''
classdocs
Copy link
Member

Choose a reason for hiding this comment

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

Describe class here. Can be same text as at top of file, but more detailed.
Worth mentioning that it is a spherical lens, and that the lens is created by a function morphing a sphere to the lens.

def checkOptions(options):
MeshType_3d_solidsphere1.checkOptions(options)
for key in [
'Spherical radius fraction',
Copy link
Member

Choose a reason for hiding this comment

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

Remove Posterior/Anterior radii from minimum zero as handled below.
I like the check options to be in the order they appear in the dictionary (except where a the same check is applied to several options e.g. if options[key] < 0.0.
I.e. check min 0.0 for 'Axial thickness', 'Spherical radius fraction'. Then min Anterior radius, min Posterior radius. Use 'for key in [anterior/posterior]' as same expression. Lastly limit Spherical radius fraction to max 1.0.

@rchristie rchristie merged commit 6853716 into ABI-Software:master Oct 1, 2018
@mlin865 mlin865 deleted the lens branch October 1, 2018 04:13
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.

2 participants