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

Add tests for the Nix build #791

Merged
merged 11 commits into from Jan 10, 2023
Merged

Add tests for the Nix build #791

merged 11 commits into from Jan 10, 2023

Conversation

wd15
Copy link
Contributor

@wd15 wd15 commented Mar 22, 2021

Seems to be possible to run completely independent configurations on Travis with the jobs / include syntax.

Allow a Nixpkgs version tag to be passed at the command line. Update
the version of Nix to 20.09.
Seems to be possible to run completely separate configurations using
the jobs -> include syntax. Trying with a Nix build.
@wd15
Copy link
Contributor Author

wd15 commented Mar 23, 2021

Only seems to be running a single version of the Travis osx tests

@wd15
Copy link
Contributor Author

wd15 commented Mar 24, 2021

The following is failing on the Nix build so need to fix that. Also failing locally. Might be to do with the Gmsh version. Should be able to get to the bottom of this one.

======================================================================
FAIL: circleQuad (examples.diffusion)
Doctest: examples.diffusion.circleQuad
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/nix/store/z65l1jqvxa58zzwwa3bvglb6asj4y8cv-python3-3.8.5/lib/python3.8/doctest.py", line 2204, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for examples.diffusion.circleQuad
  File "/home/travis/build/usnistgov/fipy/examples/diffusion/circleQuad.py", line 0, in circleQuad
----------------------------------------------------------------------
File "/home/travis/build/usnistgov/fipy/examples/diffusion/circleQuad.py", line 159, in examples.diffusion.circleQuad
Failed example:
    print(phi.allclose(x, atol = 0.035)) # doctest: +GMSH
Expected:
    1
Got:
    False

The other failures on Travis aren't to do with these changes, but VTK. Getting

numpy.dtype(numpy.character): vtkConstants.VTK_UNSIGNED_CHAR,
    DeprecationWarning: Converting `np.character` to a dtype is deprecated. The current result is `np.dtype(np.str_)` which is not strictly correct. Note that `np.character` is generally deprecated and 'S1' should be used.

@guyer
Copy link
Member

guyer commented Mar 24, 2021

Can you tell if it's a different version of Gmsh or something?

@wd15
Copy link
Contributor Author

wd15 commented Mar 24, 2021

Can you tell if it's a different version of Gmsh or something?

It's giving

$ gmsh --version      
4.6.0

Is that supported?

@guyer
Copy link
Member

guyer commented Mar 24, 2021

$ gmsh --version      
4.6.0

Is that supported?

It looks like it stopped working between Gmsh 4.4 and 4.5.

I think that failure shouldn't stop you from merging this PR.

@wd15
Copy link
Contributor Author

wd15 commented Mar 24, 2021

It looks like it stopped working between Gmsh 4.4 and 4.5.

I think that failure shouldn't stop you from merging this PR.

I'll try and fix it. This is an issue that arose from introducing a newer version of Gmsh than Conda's (or whatever FIPy conda recipe is using) version so should be fixed. Is Gmsh fixed in FiPy's conda recipe? At least one of the conda tests is using 3.06.

@guyer
Copy link
Member

guyer commented Mar 25, 2021

I think the "fix" is just to relax the tolerance of the test. We definitely should support Gmsh 4.x and, largely, we do.1 I've often seen we get sporadic failures when different versions of Gmsh tweak their meshing algorithms. We either need to find a more robust metric to test, or just (continue to) drop the tolerances.

1 Remaining issues with Gmsh 4.x

@wd15 wd15 marked this pull request as ready for review November 8, 2022 21:04
@wd15 wd15 changed the title Add Nix build to Travis Add tests for the Nix build Nov 9, 2022
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@wd15
Copy link
Contributor Author

wd15 commented Nov 9, 2022

@guyer, @tkphd: please review this when you get a chance. Running Jupyter using Nix on macos should work now.

shell.nix Outdated
pkgs = import (builtins.fetchTarball "https://github.com/NixOS/nixpkgs/archive/${tag}.tar.gz") {};
pypkgs = pkgs.python3Packages;

nixes_src = builtins.fetchTarball "https://github.com/wd15/nixes/archive/9a757526887dfd56c6665290b902f93c422fd6b1.zip";
Copy link
Contributor

@tkphd tkphd Dec 8, 2022

Choose a reason for hiding this comment

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

What is this magic tarball zip file, and is a user account the proper source for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. It's a bunch of Jupyter dependencies, here. I don't know why I thought that was a good idea. They can just be listed directly here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

[TODO]: check if nixpkgs has some sort of built in jupyter-complete expression with all Jupyter add ons.

Stop using remote repository for Jupyter install with Nix.
@wd15 wd15 requested a review from tkphd December 14, 2022 19:48
@wd15
Copy link
Contributor Author

wd15 commented Dec 14, 2022

@tkphd should be better now

Copy link
Contributor

@tkphd tkphd left a comment

Choose a reason for hiding this comment

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

LGTM!

@guyer
Copy link
Member

guyer commented Jan 10, 2023

Remaining CI failure is due to PETSc 3.18 or some CI glitch (for example, why is the Test log blank, when it can be seen along the way that the tests run successfully to completion?). Regardless, it fails on master, too, so no reason to hold up this PR.

@guyer guyer merged commit db5292d into usnistgov:master Jan 10, 2023
@wd15 wd15 deleted the update-nix branch August 7, 2023 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants