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

ENH: Enable querying FEM values from Python #4398

Merged
merged 7 commits into from
Sep 13, 2023

Conversation

matthewturk
Copy link
Member

This is a very minor function wrapper to enable querying FEM values from within Python.

@neutrinoceros
Copy link
Member

I don't think I have necessary expertise to review this, but maybe it should be advertised to someone who does ?

@matthewturk
Copy link
Member Author

matthewturk commented Apr 18, 2023 via email

@neutrinoceros
Copy link
Member

Could you show an example of how it's used ? I don't know this module at all so it's hard for me to guess

@matthewturk
Copy link
Member Author

Hi! Just as a note, I do want to return to this in the next 24 hours. I tried to find my big example script but was unsuccessful. Trying again. :)

@chrishavlin chrishavlin self-assigned this Aug 25, 2023
@chrishavlin
Copy link
Contributor

I can take a look at this!

Copy link
Contributor

@chrishavlin chrishavlin left a comment

Choose a reason for hiding this comment

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

Generally looks great, found one line that might need adjusting.

And in testing things out in the notebook here, I ended up generally needing both map_reals_to_unit and sample_at_real_points. Since the sampling itself does not enforce element bounds, I needed map_reals_to_unit to build a mask so I could fill in empty values to the array I got back from sample_at_real_points. And since check_inside isn't available at the python level, I had to go check the code to remind me what the proper element coordinate ranges were for the various elements... so I think it could be useful to have a way to call check_inside at the python level as well (that works with arrays the functions you've already added), or maybe an option for sample_at_real_points that would call check_inside before sampling and fill (with nan?) otherwise? what do you think?

yt/utilities/lib/element_mappings.pyx Outdated Show resolved Hide resolved
@matthewturk
Copy link
Member Author

otherwise? what do you think?

This sounds like a good idea. I'll implement it!

@matthewturk
Copy link
Member Author

@chrishavlin What do you think of this change?

Copy link
Contributor

@chrishavlin chrishavlin left a comment

Choose a reason for hiding this comment

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

nice! the new function is great (after a small fix)!

yt/utilities/lib/element_mappings.pyx Outdated Show resolved Hide resolved
yt/utilities/lib/element_mappings.pyx Outdated Show resolved Hide resolved
matthewturk and others added 3 commits September 12, 2023 11:43
Co-authored-by: Chris Havlin <chris.havlin@gmail.com>
Co-authored-by: Chris Havlin <chris.havlin@gmail.com>
chrishavlin
chrishavlin previously approved these changes Sep 12, 2023
@neutrinoceros
Copy link
Member

@matthewturk is this still WIP ? I think it can go in as soon as the WIP check and limiting are green

@matthewturk
Copy link
Member Author

pre-commit.ci autofix

@matthewturk matthewturk added the enhancement Making something better label Sep 12, 2023
@neutrinoceros
Copy link
Member

@yt-fido test this please

1 similar comment
@neutrinoceros
Copy link
Member

@yt-fido test this please

@neutrinoceros neutrinoceros added this to the 4.3.0 milestone Sep 13, 2023
@neutrinoceros neutrinoceros merged commit 8757cf2 into yt-project:main Sep 13, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Making something better index: unstructured mesh new feature Something fun and new!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants