Skip to content

WIP: Towards working CI.#227

Closed
wcwitt wants to merge 6 commits intomainfrom
towards-working-ci
Closed

WIP: Towards working CI.#227
wcwitt wants to merge 6 commits intomainfrom
towards-working-ci

Conversation

@wcwitt
Copy link
Collaborator

@wcwitt wcwitt commented Aug 22, 2024

See #226. I'm creating this PR to trigger the CI. I won't merge without confirmation from someone.

@wcwitt wcwitt mentioned this pull request Aug 22, 2024
@wcwitt
Copy link
Collaborator Author

wcwitt commented Aug 22, 2024

IMPORTANT. I've disabled the experimental tutorial because I don't know how to fix it myself. Need to revive it before this is merged.

Otherwise, I am making progress. @tjjarvinen do you understand this documentation error? I think it's this but not certain.

These are docstrings in the checked modules (configured with the modules keyword)
that are not included in canonical @docs or @autodocs blocks.

@cortner
Copy link
Member

cortner commented Aug 22, 2024

Doesn't this just mean there are new functions defined with doc strings and they should be listed in the docs. I think I had this happen somewhere else? (I might misremember...)

@wcwitt
Copy link
Collaborator Author

wcwitt commented Aug 22, 2024

It does seem that way, but we definitely don't mention all functions by name in the docs; instead there is some automatic feature that finds them. Why can't it find these? I need to remind myself of the details.

Reexport = "1"
StaticArrays = "1"
YAML = "0.4"
julia = "~1.10.0"
Copy link
Member

Choose a reason for hiding this comment

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

I thought the ~ means that we restrict to 1.10.x and don't allow 1.11. This was deliberate. Did I misunderstand something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Everything in this PR is up for discussion, no question. But this CI failure on nightly (which is running 1.12.0-DEV) was caused by the ~, as best I can tell, and I would prefer to always have the CI passing on main.

https://github.com/ACEsuit/ACEpotentials.jl/actions/runs/10500352832/job/29088598779

Copy link
Member

Choose a reason for hiding this comment

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

Then turn off nightly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can live with that 0c1b7cc


[compat]
ACEpotentials = "0.6"
ACEpotentials = "0.8"
Copy link
Member

Choose a reason for hiding this comment

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

Should there even be a compact entry? Shouldn't the docs build just dev the "current" ACEpotentials?

Copy link
Member

Choose a reason for hiding this comment

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

This is the line I get when building docs via CI:

  [3b96b61c] + ACEpotentials v0.8.0-dev `~/work/ACEpotentials.jl/ACEpotentials.jl`

So the compat entry in the docs/Project folder is really not needed (and shouldn't be there)

Pa2 = algebraic_smoothness_prior(model.basis; p=2)
Pa4 = algebraic_smoothness_prior(model.basis; p=4)
Pg = gaussian_smoothness_prior( model.basis, σl = (2/r_nn)^2, σn = (0.5/r_nn)^2);
Pa2 = ACEpotentials.algebraic_smoothness_prior(model.basis; p=2)
Copy link
Member

Choose a reason for hiding this comment

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

I think this is wrong. It should be ACE1x.algebratic_smoothness_prior etc .... But we are almost ready to get rid of ACE1 and ACE1x entirely so I'm not sure it's worth working on this right now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure, I chose ACEpotentials because of this

algebraic_smoothness_prior(model; p = 4, wl = 2/3, wn = 1.0) =

I don't mind changing to ACE1x if that's better, but at minimum the change I made allows that part of the docs to build successfully, where it failed before.

I don't care about this example specifically, but I want to fix the unimportant CI failures so that more impactful things don't get lost.

@cortner cortner mentioned this pull request Aug 23, 2024
@wcwitt
Copy link
Collaborator Author

wcwitt commented Aug 23, 2024

Closing, replaced by #229.

@wcwitt wcwitt closed this Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants