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

BUG: Assign domain dims after updates in Athena #4815

Merged
merged 2 commits into from
Feb 21, 2024

Conversation

matthewturk
Copy link
Member

As reported on the mailing list, in 2D athena datasets we can't modify the domain_dimensions (which are a MutableAttribute) after setting them. So, we hold off on setting them until after we do the dimensionality updates.

@matthewturk matthewturk added bug code frontends Things related to specific frontends backport-stable on-merge: backport to stable labels Feb 11, 2024
neutrinoceros
neutrinoceros previously approved these changes Feb 11, 2024
Copy link
Member

@neutrinoceros neutrinoceros left a comment

Choose a reason for hiding this comment

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

Having a hard time understanding how this wasn't caught earlier but otherwise LGTM !

@neutrinoceros neutrinoceros added this to the 4.3.1 milestone Feb 11, 2024
@neutrinoceros neutrinoceros removed the backport-stable on-merge: backport to stable label Feb 11, 2024
@neutrinoceros
Copy link
Member

(I don't remember what the "backport-stable" label is suppose to do but I'm sure it's not how we've been doing backports lately. I've set the milestone to backport to 4.3.x)

@matthewturk
Copy link
Member Author

@neutrinoceros thanks, I wasn't sure!

I think because we don't test 1 or 2D athena datasets we didn't catch it. Mocking them would help, but is likely extremely difficult to set up.

The thing about MutableAttribute, to my reading, is that it generally could provide us a lot of being able to avoid conversions etc, but we don't yet take advantage of it. I think in one of the optimization PRs I tried to. I should return to that.

@neutrinoceros
Copy link
Member

Blocked by #4814

@jzuhone
Copy link
Contributor

jzuhone commented Feb 11, 2024

@yt-fido test this please

1 similar comment
@jzuhone
Copy link
Contributor

jzuhone commented Feb 11, 2024

@yt-fido test this please

jzuhone
jzuhone previously approved these changes Feb 12, 2024
@cphyc
Copy link
Member

cphyc commented Feb 15, 2024

The failing tests are unrelated and fixed in #4819.

@cphyc cphyc disabled auto-merge February 15, 2024 09:14
@cphyc cphyc enabled auto-merge February 15, 2024 09:15
@neutrinoceros
Copy link
Member

This is blocked by an unresponsive bot. Can we put it to sleep ?

@matthewturk
Copy link
Member Author

Yes

@wkal2
Copy link

wkal2 commented Feb 15, 2024

Hi all, Many thanks for your attention to the domain dimensions issue I have! After trying the pull request, I have however encountered another error, which I have quoted below

Traceback (most recent call last):
File "/home/wkal2/rds/hpc-work/project/code/plot_full_output.py", line 17, in
ds = yt.load(vtk_filename,parameters=parameters)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/wkal2/yt/yt/_maintenance/deprecation.py", line 69, in inner
return func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^
File "/home/wkal2/yt/yt/loaders.py", line 145, in load
return cls(fn, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/wkal2/yt/yt/frontends/athena/data_structures.py", line 502, in init
self.index
File "/home/wkal2/yt/yt/data_objects/static_output.py", line 606, in index
self._instantiated_index = self._index_class(
^^^^^^^^^^^^^^^^^^
File "/home/wkal2/yt/yt/frontends/athena/data_structures.py", line 134, in init
GridIndex.init(self, ds, dataset_type)
File "/home/wkal2/yt/yt/geometry/geometry_handler.py", line 39, in init
self._setup_geometry()
File "/home/wkal2/yt/yt/geometry/grid_geometry_handler.py", line 41, in _setup_geometry
self._parse_index()
File "/home/wkal2/yt/yt/frontends/athena/data_structures.py", line 335, in _parse_index
self.dataset.domain_dimensions[2] = 1
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^
ValueError: assignment destination is read-only
Exception ignored in: <function GridIndex.del at 0x14f7824e3420>
Traceback (most recent call last):
File "/home/wkal2/yt/yt/geometry/grid_geometry_handler.py", line 67, in del
del self.grids
^^^^^^^^^^
AttributeError: 'AthenaHierarchy' object has no attribute 'grids'

So sorry if you have already received this message on other platforms and really appreciate your help and assistance thus far, do let me know if there is anything I could do on my part!

@matthewturk
Copy link
Member Author

@wkal2 I've updated the PR with a slightly different change. Can you try this?

@wkal2
Copy link

wkal2 commented Feb 16, 2024

Thanks! Just tried to run the code again but this time there is a module error

Traceback (most recent call last):
File "/home/wkal2/rds/hpc-work/project/code/plot_full_output.py", line 8, in
import yt
File "/home/wkal2/yt/yt/init.py", line 14, in
from yt.data_objects.api import (
File "/home/wkal2/yt/yt/data_objects/api.py", line 1, in
from . import construction_data_containers as __cdc, selection_objects as __sdc
File "/home/wkal2/yt/yt/data_objects/construction_data_containers.py", line 17, in
from yt.data_objects.selection_objects.data_selection_objects import (
File "/home/wkal2/yt/yt/data_objects/selection_objects/init.py", line 1, in
from .boolean_operations import (
File "/home/wkal2/yt/yt/data_objects/selection_objects/boolean_operations.py", line 5, in
from yt.data_objects.selection_objects.data_selection_objects import (
File "/home/wkal2/yt/yt/data_objects/selection_objects/data_selection_objects.py", line 14, in
from yt.data_objects.data_containers import YTDataContainer
File "/home/wkal2/yt/yt/data_objects/data_containers.py", line 13, in
from yt.data_objects.profiles import create_profile
File "/home/wkal2/yt/yt/data_objects/profiles.py", line 5, in
from yt.fields.derived_field import DerivedField
File "/home/wkal2/yt/yt/fields/derived_field.py", line 18, in
from .field_detector import FieldDetector
File "/home/wkal2/yt/yt/fields/field_detector.py", line 7, in
from yt.utilities.io_handler import io_registry
File "/home/wkal2/yt/yt/utilities/io_handler.py", line 10, in
from yt.geometry.selection_routines import GridSelector
ModuleNotFoundError: No module named 'yt.geometry.selection_routines'

@matthewturk
Copy link
Member Author

That looks unrelated, and is usually because you need to recompile the yt compiled bits.

@wkal2
Copy link

wkal2 commented Feb 16, 2024

My bad, it is working now, thank you so much!

@chrishavlin
Copy link
Contributor

This looks good to merge, ya? @neutrinoceros and @matthewturk

@cphyc cphyc merged commit 56346fb into yt-project:main Feb 21, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug code frontends Things related to specific frontends
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants