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

Fix import #57

Closed
ddundo opened this issue Nov 17, 2024 · 4 comments · Fixed by #59
Closed

Fix import #57

ddundo opened this issue Nov 17, 2024 · 4 comments · Fixed by #59
Assignees
Labels
bug Something isn't working testing Extensions and improvements to the testing infrastructure

Comments

@ddundo
Copy link
Member

ddundo commented Nov 17, 2024

While looking at mesh-adaptation/docs#59, I realised that the test_import.py test is not actually doing what it's meant to do.

Edit: I opened a PR which I thought fixes it, but then realised the actual issue... When you do python3 -m pytest tests/test_import.py from firedrake/src/UM2N, the test passes (as in the CI test suite). But it fails if you do python3 -m pytest test_import.py from firedrake/src/UM2N/tests. The former also fails if you do pytest tests/test_import.py.

So I guess python path might not be set correctly during installation?

@ddundo ddundo self-assigned this Nov 17, 2024
@ddundo ddundo added bug Something isn't working testing Extensions and improvements to the testing infrastructure labels Nov 17, 2024
ddundo added a commit that referenced this issue Nov 17, 2024
This was referenced Nov 17, 2024
@ddundo ddundo changed the title Fix test_import.py Fix import Nov 17, 2024
@jwallwork23
Copy link
Member

It seems to be okay in #59?

@jwallwork23 jwallwork23 moved this from In Progress to In review in Mesh Adaptation development board Nov 18, 2024
@ddundo
Copy link
Member Author

ddundo commented Nov 18, 2024

It seems to be okay in #59?

Well that uninstalls and then installs UM2N again. So maybe that fixes it, yeah. But could you please try it in the docker container as it is and see if you get the same error?

@jwallwork23
Copy link
Member

Well that uninstalls and then installs UM2N again. So maybe that fixes it, yeah. But could you please try it in the docker container as it is and see if you get the same error?

Sorry, it was a long day. I started digging into this, will get back to it later in the week. I think there's an issue with dependencies.

@ddundo
Copy link
Member Author

ddundo commented Nov 19, 2024

Sorry, it was a long day. I started digging into this, will get back to it later in the week. I think there's an issue with dependencies.

No worries, no rush at all from my side :)

jwallwork23 added a commit to mesh-adaptation/movement that referenced this issue Nov 23, 2024
jwallwork23 added a commit that referenced this issue Nov 23, 2024
Not strictly required for #57 but related work.

`meshio` and `tensorboardX` are never actually used in the code.

`torch` is used, but it should be installed for the desired architecture
before attempting to install UM2N.
jwallwork23 added a commit that referenced this issue Nov 23, 2024
Closes #57.

Linked PR:
mesh-adaptation/docs#59.

This PR sets up UM2N's CI to use the newly added bespoke Docker image.
It also makes use of the reusable test workflow from the docs repo.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working testing Extensions and improvements to the testing infrastructure
Development

Successfully merging a pull request may close this issue.

2 participants