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

Added a dummy tag for ForwardDiff configs being constructed in Gridap Autodiff.jl to fix issue #805 #806

Merged
merged 4 commits into from
Jun 23, 2022
Merged

Added a dummy tag for ForwardDiff configs being constructed in Gridap Autodiff.jl to fix issue #805 #806

merged 4 commits into from
Jun 23, 2022

Conversation

kishore-nori
Copy link
Collaborator

No description provided.

@amartinhuertas amartinhuertas self-requested a review June 22, 2022 11:40
Copy link
Member

@amartinhuertas amartinhuertas left a comment

Choose a reason for hiding this comment

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

@kishore-nori
Copy link
Collaborator Author

kishore-nori commented Jun 23, 2022

After making the last commit of changes the CI takes

Test Summary:  | Pass  Total
FESpaces (2/2) |  855    855
1699.394610 seconds (472.28 M allocations: 26.718 GiB, 0.88% gc time, 99.82% compilation time)

with a dummy tag function it took:

Test Summary:  | Pass  Total
FESpaces (2/2) |  855    855
516.246620 seconds (387.76 M allocations: 21.898 GiB, 1.95% gc time, 99.66% compilation time)

I think this is caused by the tag checking now (also having long tag's like we are ending up here may be has to do something with this, I am not sure, will have to look into this, haven't got much details from profiler)(sorry the tag check is not happening at all here, due to low-level API usage, the next or below might be the reason). Or it could be I am doing something horribly bad in the last commit (I don't feel so, as the changes have been very minimal, just adding a function to existing structs to extract the tag name while setting up ForwardDiff configs)

Update: Digging through the code, I also feel this could be because of type instability in inferring the return of type of _change_argument (the ForwardDiff tag is now being generated from the output of _change_argument, this might be propagating the type instability of pre-existing type-instability of _change_argument), may be better to type annotate the output of the _change_argument function as Function, I am not entirely sure if this can help.

@amartinhuertas amartinhuertas merged commit e27c6bb into gridap:master Jun 23, 2022
@kishore-nori kishore-nori deleted the AD-for-SkeletonIntegration branch June 23, 2022 13:13
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