Skip to content

Conversation

@kaelyndunnell
Copy link
Contributor

@kaelyndunnell kaelyndunnell commented Feb 26, 2025

Proposed changes

Adds export for average surface temperature.

Types of changes

What types of changes does your code introduce to FESTIM?

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code refactoring
  • Documentation Update (if none of the other choices apply)
  • New tests

Checklist

  • Black formatted
  • Unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

Copy link
Collaborator

@jhdark jhdark left a comment

Choose a reason for hiding this comment

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

Just some small doc fixes from me, I think this class should probably inherit from AverageSurface then might be able make it simpler.

Going back to this is making me realize we need to have a more dev team encompassing discussion on how these derived are handled in general. But this one can probably still go through for now

Comment on lines 35 to 51
def compute(self, ds):
"""Computes the average temperature on the surface.
Args:
ds (ufl.Measure): surface measure of the model
"""
temperature_field = self.temperature_field

surface_integral = fem.assemble_scalar(
fem.form(temperature_field * ds(self.surface.id))
) # integral over surface

surface_area = fem.assemble_scalar(fem.form(1 * ds(self.surface.id)))

self.value = surface_integral / surface_area # avg temp

self.data.append(self.value)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This might not be needed if we inherit from AverageSurface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we still do cause this will fail if field isn't of type F.Species (even setting it to None in init made things fail)

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

Labels

enhancement New feature or request fenicsx Issue that is related to the fenicsx support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants